tenforwardconsulting / style_guide

Ten Forward Style Guide
0 stars 0 forks source link

Service Object naming #3

Open samsonasu opened 7 years ago

samsonasu commented 7 years ago

I prefer ThingThatDoesStuff.do_stuff although we've seen DoesStuff.perform and ThingThatDoesStuff.perform too. We should come up with a preferred way to handle this and codify it.

votes?

boatrite commented 7 years ago

Vote for: DoesStuff.perform

petitJAM commented 7 years ago

I like StuffDoer because you are instantiating a thing that does stuff.

StuffDoer.new(params).perform

I think if we call it DoesStuff, we shouldn't instantiate it and just use a class method

DoesStuff.perform(params)

We like never actually care about the state inside these things anyway, since we always do StuffDoer.new().perform()


So I voted for both sides, but DoesStuff is a conditional vote

petitJAM commented 7 years ago

Although, I'm not sure I'm a huge fan of DoesStuff.perform(params), maybe there's a nicer way to do that? I don't want an actual object, just a function

samsonasu commented 7 years ago

My preference is instantiating the object, since we do sometimes care about side effects, eg in ClientCreator in habit you can reach in and snag the trainer_user if you want. I like the descriptive method names too because then you can have more flexibility, eg to write both a do_stuff and a do_stuff! method which can be helpful in keeping very similar code organized in the same place.

1) StuffDoer.new(params).do_stuff 2) StuffDoer.do_stuff(params) 3) StuffDoer.new(params).perform 4) StuffDoer.perform(params) 5) DoesStuff.perform(params) 6) DoesStuff.new(params).perform

Is that is? I don't think anyone has advocated for DoesStuff.do_stuff my vote is for #1, and if you want to write static helper methods eg if it doesn't take any arguments then go for it.

Nonsense331 commented 7 years ago

My vote is for StuffDoer.new(params).do_stuff. I think we should treat them like models. Call the action you want to do on it to do that action. Return true or false from it like you would expect. If you want the errors or the model call stuff_doer.model or stuff_doer.errors.

samsonasu commented 7 years ago

return values are an important part of this, good call. I like returning true/false (or raising an exception for the bang methods).

boatrite commented 7 years ago

I think calling the class StuffDoer (or any variation) and also calling the method do_stuff is repetitious. I prefer ImportsUsers.new.perform or UserImporter.new.perform vs ImportsUsers.new.import_users or UserImporter.new.import_users. I also like the way StuffDoer (or any variation) and perform codifies the point that these are only supposed to do a single thing.

I also like returning true/false for non-bang and raising exceptions for the bang method.

I think I also like being able to call errors. We should decide what form that takes. Array of strings seems most natural. That differs from what the ActiveRecord objects returns for errors but I don't think that matters.

hilarysk commented 7 years ago

Long-time reader, first-time poster - I second Brett's comment.