Question

In my app, we have Users who can perform actions on one another - like poking on Facebook.

I'm writing the methods just now and am not really sure which approach to take. I mean, I know they're both acceptable but is there a more idiomatic approach?

Option 1

if @current_user.may_poke?(@other_user)
  @current_user.poke!(@other_user)
end

Option 2

if @current_user.may_poke?(@other_user)
  @other_user.poke!(@current_user)
end

The first option reads better in English, almost perfectly as a sentence. The second option makes more sense in terms of method naming, "poke" is a method being performed on the @other_user. The @current_user is just an argument to provide extra info - who did the poking.

Was it helpful?

Solution

I'd stick with Option 1 there.. if you follow the logic of the conditional, then you're asking "If current_user may poke other_user, then have current_user poke other_user". Option 2 doesn't make much sense when thought of in those terms.

Also.. matz, the author of Ruby, states that "The bang [exclamation point] sign means "the bang version is more dangerous than its non bang counterpart; handle with care"." ( http://www.ruby-forum.com/topic/176830#773946 ). I'd probably just use poke for that method name instead of poke!.

OTHER TIPS

The first option seems more correct, since may_poke? indicates performing an action. For the second option use the name pokable? (or pokeable?, correct spelling for this anyone?) to indicate an attribute of itself.

I say go with the option that you understand the fastest by reading it. It makes the code more readable and thus improves it (IMO).

In my case (for this specific situation), I would have gone with Option 1.

If poke is something that changes the state of @other_user, then go with option 2. But the method name poke suggests the receiver and the argument are the other way around, so to avoid confusion, change the name to something like poke_by. I don't think it is good to have the receiver and the argument reversed for may_poke. Perhaps it is better to have it in the same way:

if @other_user.pokable_by?(@current_user)
  @other_user.poke_by(@current_user)
end
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top