DEV Community

[Comment from a deleted post]
Collapse
 
trevoke profile image
Aldric Giacomoni

There's a lot of information in there.

In the order in which they're encountered in this article, my thoughts:

  1. Avoid module/class-level methods as much as possible unless they're basically factories - class-level methods make it much harder to write reasonable mocks later on, and they also make it harder to refactor the code later.
  2. DO use initializable objects! Extracting service objects that get initialized is what I do, in fact I even use that same .new + #call pattern, which is very idiomatic Ruby.
  3. Don't use a Build.call module+method technique, just use a regular class-level method for factories (in fact, turns out for this use case, we have .new...) Also in this example, there's a database call in the service object, probably a bad idea from a performance standpoint when the app grows enough.
  4. The guard clauses are a good tool, I do however use them exclusively when providing quick ways to get out of a code path, they shouldn't be used for action at all; here I would argue that:

a. the first guard clause makes sense, though I would prefer a different name for the object we are calling, as the responsibility at this point in the flow is to check that incoming data is sane, so it doesn't completely make sense to say that we are validating a user, unless we're actually using the Rails approach which would involve User.new(params).valid? -- since we're in Rails, nothing wrong with initializing an object and telling it to do something.
b. the second guard clause should just be the if/else that indicates the main success/failure case of the controller action being called, it shouldn't read like they are different code paths; in addition, for 99% of Rails apps, the built-in view rendering system is good enough and there's no reason to implement another one.