search

visionmedia / express-resource

Resourceful routing for Express
1.41k stars 140 forks source link

Default ID should be 'id', not based on name #26

Open aseemk opened 13 years ago

aseemk commented 13 years ago

Just spent a bunch of time debugging what turned out to be caused by us reusing some code that said req.params.id for a non-top-level resource.

IMHO, having the API change based on what resource you're in doesn't exactly meet the principle of least surprise. =D

Also, FWIW, from a usage perspective, id is a much better variable name because it's actually a string, whereas e.g. forum or w/e isn't accurate, because it's not a Forum object (or whatever my model-layer objects are).

So here's my ask for the default ID to be consistent and simple. You could still pass in a custom ID as an option, etc.

tj commented 13 years ago

ah, the joys of controller abstractions :D (this kind of thing is why I hate rails). I dont disagree with the id thing, though if/when others are involved switching to forum or forum_id would be annoying

pacovell commented 13 years ago

@aseemk, can you be more specific about where you ran into the problem?

I agree that the 'current resource' ID should always be 'id'. But, if you are nested, then your parent 'id's need to be called something else. I generally expect that, for example, '/forums/12/threads/15' in the thread controller you will have id === 15 and forum_id === 12. Is this what you mean?

@TJ, I don't see how you get away from a controller of some sort for a resource. You need something to bind the incoming request with the underlying data models while applying authentication/authorization controls and rendering the proper result. You don't have to call it a controller, but somewhere you want that code.

tj commented 13 years ago

@pacovell sure, to a certain degree yeah, js just does not have an "elegant" way to express this stuff. when you get away from object literals things get so ugly you might as well just use app.get() directly and benefit from clarity

panosru commented 12 years ago

I prefer the current way, besides if all resource params will be named as id then how we will apply [Route Param Pre-conditions](http://expressjs.com/guide.html#route-param pre-conditions)?

I think it is more clear as it is now.