← Back to Upcase

A comment about SRP on the source code of learn


(Guirec Corbel) #1

Hi,

Again about responsability, in this code, you enable to the Purchase model to create a user. I think it’s a little bit strange to do purchase.create_user. Am I right? Is it a reason to do this like this?

Thanks!


(Derek Prior) #2

Yes, it is not ideal for the Purchase model to have knowledge of how to create a user object. I know very little about this particular application, but it strikes me that this should be the work of some other object that takes a purchase and creates or finds an existing user for it or something like that.

What you’re seeing is the result of a codebase that has grown and evolved over time. It’s got some rough spots.


(Guirec Corbel) #3

I’m glad to see than you don’t do a perfect code the first time. I always questioning myself if I do a good code and I have many, many, many question each times I do something. To see than experts are humans too is reassuring.

Do you think than refactoring must be done if there is no problem? When do you think we must stop to refactor and just continue to develop? In this particular case, there is no problem and everything works. Will you change it?


(Ben Orenstein) #4

All good questions.

I like to refactor at a few specific times:

  1. When I just got a test passing. This is part of the red-green-refactor loop.
  2. When I need to change code. If I’m finding the code hard to change, I’ll first try to refactor it to make the change easy to make.
  3. When I found a bug in some code. That means the code was too hard to read (since I couldn’t see the bug), so I refactor to improve readability.

You’ll notice that none of these say “refactor whenever you find some bad code”. Occasionally, I’ll refactor some bad code I find just so that it’s better, but usually I wait until there’s a good reason to do so.