eGT-Labs / egt-gsa-proto

This will be the repo for Pool 2 - GSA Agile Delivery Services Prototype
0 stars 0 forks source link

Backend code review #97

Closed mpandurangi closed 9 years ago

mpandurangi commented 9 years ago

Code review for backend

jaalfaro commented 9 years ago

Some things to consider

  1. Routes is usually the declarations of the routes and calls into your business logic elsewhere
  2. I’d pull your cache into its own module, and maybe even look at structuring it as a service that can use multiple backers. Right now, you are doing in memory caching, and while that’s great now, it doesn’t scale well where you could use pluggable providers that move it to something like redis or some other more permanent storage.
  3. I would look at moving the promise logic out into its own module. Its the core of your business logic, and the way its written, it almost feels like you threw it in so you could say I can do promises. Where if you move it into its own module, it looks more like a patten that you plan to follow.
  4. eventCountCache – Maybe set this as configurable, if I read the docs right, your default of 1M could blow out quickly, but as you get bigger, you will have to start to worry about memory. Not sure if any testing was done to see how well this works, and I’ve not looked at the size of the responses from the OpenFDA, but they look pretty big, so you may not be storing much in there, and with a cache period of 1 week, I’d think you were expecting some savings.
jehanson commented 9 years ago

Addressed in pull request #105