3

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.

bodacious
  • 6,608
  • 9
  • 45
  • 74

4 Answers4

4

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!.

Patrick Lewis
  • 2,131
  • 2
  • 17
  • 16
  • My thinking was to add the **bang** since the User object is modified within the method. i.e. Column updates are applied rather than just the attributes being updated in memory. – bodacious Feb 12 '13 at 16:25
2

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.

PinnyM
  • 35,165
  • 3
  • 73
  • 81
1

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.

Phil
  • 3,934
  • 12
  • 38
  • 62
  • 1
    Yep - I think readability is what's most important. Just wanted to make sure this wasn't already defined in some principle of OO that I hadn't come across or had forgotten :) – bodacious Feb 12 '13 at 16:24
  • 1
    Kudos to you for seeing the bigger picture :) – Phil Feb 13 '13 at 10:32
0

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
sawa
  • 165,429
  • 45
  • 277
  • 381