search

firstdraft / draft_generators

Rails generators that help beginners learn to program.
MIT License
2 stars 3 forks source link

Changing the variable names in Index and Create #96

Open pmckernin opened 4 years ago

pmckernin commented 4 years ago

Right now the variable we are using in the index action is

matching_photos = Photo.all

    @list_of_photos = matching_photos.order({ :created_at => :desc })

I think we should change this variable from matching_xxx to all_xxx, because currently, we are not matching anything.

Also, the variable we are using for the create action currently is the_xxxx, I think we should change it to new_xxxx. I think that this might be friendly for new students.

@raghubetina @jelaniwoods What are your thoughts?

jelaniwoods commented 4 years ago

@pmckernin I would argue that we are in fact matching everything. However, I can see the advantage of using .where to get matching records and .all to get all records.

I think it could be beneficial, but I don't think it's urgent– considering we've been doing matching-abc all quarter I think.

raghubetina commented 4 years ago

I would argue that we are in fact matching everything.

☝🏾 that.

Also, I take your point about the suboptimal name in the index action @pmckernin , but the motivation is to help ease the confusion around needing to .at(0) later in the show et al actions; i.e., getting them used to the idea that .where returns a Relation. So the variable name is supposed to be a bridge between the return class of .all and that of .where, since it seems more clear and obvious that .all returns multiple things.

I am open to changing the variable name, but I think it should be consistent between actions, whatever it is (as long as it contains Relations). I.e., we could consider result_set or query_results or something like that. Does that sound better?