Humans Present: Refactoring

Upcase
1
This is a companion discussion topic for the original entry at https://thoughtbot.com/upcase/videos/humans-present-refactoring
2

Hi,

I just watched the video “Humans Present: Refactoring”. I realy love this kind of videos. This is experts but they looks like humans. It’s the real life without montage like Railscasts, Destroy all software or Ruby Tapas and the subject is very interessant. I’m happy to see how experts work on a real rails project.

I have some comments :

  • Why to use observers instead service objects?
  • You use @subscribable.user.email. What about demeter? Is it better to something like @subsribable.email_receiver?
  • What about the SRP? You use some includes to remove duplications but I think activerecords models’s responsability is the persitence and the validation (the interaction with the database). What’s is your point of view?
  • Is it an open-source project? Is it possible to view the final code?

Thanks For the video. I will be happy if there is more like this in the futur.

3

I think the code is from https://www.apptrajectory.com/

This, AFAIK, is not open source.

4

Hi, Guirec Corbel!

You questions look reasonable and I would like to get answers for them too.

5

@jferris I believe you were the “human” in this one. Any thoughts on these questions?

6

Those are all good questions! It’s been a while since I actually made those decisions, but here’s what I think:

Why to use observers instead service objects?

Moving to service objects would be a good, separate extraction. As the application existed when I started, observers and callbacks were widely used. Observers are an easy extraction from callbacks, and are probably the lesser evil when duplication is in play. Using a service object to replace this observer would be a good followup.

What about demeter?

This would be another good improvement. While refactoring, it’s common for new problems to be introduced as you remove others. As long as the new problems are less egregious and it’s clear how they can be solved in the long term, I still think these intermediate steps are useful.

What about the SRP? You use some includes to remove duplications but I think activerecords models’s responsability is the persitence and the validation (the interaction with the database). What’s is your point of view?

I love SRP, as well as the other letters of SOLID. However, you need to pick your battles and decide what to fight based on the code in front of you. There was some pretty gross duplication going on between a few large classes in the application, so any logic extracted from these models was a plus. When working on an application like this, it’s important to aggressively attempt to extract concerns you’re touching until you get close enough that you can start thinking about SRP.

7

Thanks @jferris. Very nice anwser.