This is a companion discussion topic for the original entry at https://thoughtbot.com/upcase/videos/ruby-science-extract-class
Great episode guys; @benorenstein that was creepy
@melanie @jferris I have a couple of questions though.
- Why didn’t you create a recipient list object instead of making it a method inside the
- I know this would be like arbitrary but why make the parameters list an attributes hash if there were just like two parameters?
- Isn’t better just to add an
attr_readerfrom the start so you don’t have to be adding
@like everywhere; even if you start with private accessors at first?
- Isn’t the code in the controller breaking tell, don’t ask and making decisions based on the
Thanks in advance for all your answers,
BEN IS BACK! YAY I CAN BEGIN WATCHING AGAIN STARTING NEXT WEEK!
But seriously, love all the eps this one was great.
I agree about
attr_readers, I always use them and make them
private if need be.
I immediately thought that as soon as the
valid? method was added to the
SurveyInviter class. I think it’s a good idea to move in smaller steps though and getting to a state where you can see that it violates Tell Don’t Ask. We need to be careful to keep in mind Separations of Concerns and making sure that the controller logic is still in the controller. So we’d still need a query method, something like
success? in order to redirect or render errors. I normally create Request/Response Objects that look like this:
response = InviteToSurvey.call if response.success? # happy path else @errors = response.errors render "new" end
Why didn’t you create a recipient list object instead of making it a method inside the
It’s generally good practice to do one refactoring at a time. When extracting a class, it’s not unusual to find out you can actually extract two! In the example app in Ruby Science, we ended up extracting several classes for this interaction.
I know this would be like arbitrary but why make the parameters list an attributes hash if there were just like two parameters?
One big difference between one parameter and two is that you can forget the order with two. Using named parameters makes the order unimportant.
Isn’t better just to add an
attr_readerfrom the start so you don’t have to be adding @ like everywhere; even if you start with private accessors at first?
I think there are valid points on either side of the argument:
attr_readerkeeps the abstraction flat and prevents churn if you need to change it to a method later.
@directly tells the reader explicitly that it’s an instance variable, so you don’t have to wonder how it works.
In all my time in Ruby, I’ve never found that either one of these approaches has much effect on readability or ease of change, so I’d use whichever style appeals to you aesthetically.
Isn’t the code in the controller breaking tell, don’t ask and making decisions based on the
Sometimes Separation of Concerns (MVC) and Tell, Don’t Ask don’t play well together. If you want to keep view/controller logic separate, it means you’ll sometimes have to ask the model layer questions and then make decisions on their behalf. Otherwise, you’ll have to peform rendering and other logic in your models. MVC has proven to be a win in most applications, but it’s not universal.
@aaronmcadam yeap; I’m also using that approach with the response objects; quite handy.
@jferris Thanks for all your answers; and thanks for the episode again. Keep them coming
Lightbulb moment. This is awesome!
It there a reason that you did not create tests for this new object? While the methods that we moved to the SurveyInviter had been private, and thus did not need tests, the SurveyInviter methods are public.