tedjt/crunchbase-api — pull request #1 by ivolo (2014-01-26 22:50:51+00:00)
Title: refactoring code and api

Hey - just wanted to give a bit of crit on the code and the API. There's OCD and non-OCD mixed in

- The best way to start designing a module is to write the readme first, which helps keep the API clean. its how you and other people will interact with your module.
- I renamed the prototype from `CrunchbaseApi` to `CrunchBase` because thats how they spell it in their brand with a capital `B`. The `Api` part is non-essential; since thats the main programmatic way to get the data from them. Any different way can add a suffix.
- I removed the `emitter` since its not throwing any errors outside the context of a callback. I go back and forth on this as a way to log, but feel free to add it back in if its useful.
- I moved the `apiKey` as the first argument of the prototype, and added a validity check. There's no way to use this module without a valid `apiKey` so making setting it optional and as another method is a bit confusing.
- I split the `company` call into `search`, `permalink`, and `company`. I prefer to keep the methods as doing one piece of logic, especially when that thing maps to a direct call on another API. You can also think of it from the user perspective:
- "I am searching CrunchBase for the name 'Segment.io'" => `.search('Segment.io', callback)`
- "Now that I found the company I'm looking for, I'm going to get more information via the CrunchBase permalink segment-io". => `.permalink('segment-io', callback)`
- "I want to search for company Segment.io, and get more information about the first result" (main use case) => `.company('Segment.io', callback)`
- It's weird that their API doesn't send back the proper content type `application/json` header. That's why superagent doesnt auto parse it into json as `req.body`. That `try / catch` construct took up a lot of that method so I removed it. I usually prefer hard fails, especially when I know if I know an error condition signifies that nothing else will work. If that response isnt json, or is not full, I want that code to crash because the API contract I'm relying on isnt the same anymore.
- re: argument naming: I prefer for smaller names, so `json` instead of `companyJson`. It keeps the code smaller and easier to read. The other thing is if you're forced to prepend prefixes like `companyJson` because there's two `json`s in the method, then its likely you should split up into multiple methods.

Hope this helps!