koopjs / koop-provider-agol

ArcGIS Online provider for Koop (public services only).
Other
12 stars 10 forks source link

Standardize status responses to the client #66

Closed dmfenton closed 9 years ago

dmfenton commented 9 years ago

@ngoldman This could use a solid read. I'm going to hold off on adding a lot of tests around the new download method until I refactor it further into more methods.

This PR begins to refactor the massive findItemData method and moves all status returns to a single method called _returnStatus

Some key points:

ungoldman commented 9 years ago

Generally looks okay to me.

As an aside, I think we should start moving methods out of the controller if they don't correspond to routes. Typically any exposed method on a controller should correspond 1:1 with a route or at least a route filter in express/MVC. Moving utility methods into a lib/ folder would take down the rather long line count here too.

dmfenton commented 9 years ago

Totally agree that we should factor out methods that don't correspond to routes. Good point about the utility methods.

dmfenton commented 9 years ago

@ngoldman good to go