Until what point should I refactor?

Question

What do you think until what point should a programmer refactor the code?

Basically having

def method
  do_something
end

Pieces of code would be handy, but they increase spaghetti code until the point where you have to remember the path of more than 8-10 methods. So wouldn't be easier to have no more than 3 methods spaghetti code, despite the fact that the method is longer than it suppose to be

Relevant: https://sites.google.com/site/unclebobconsultingllc/one-thing-extract-till-you-drop — Florian Margaine, Sep 13 '13 at 07:32
If you feel like you've reached your refactoring limit in Ruby then try switching to a more powerful language. Number of methods shouldn't be a concern, relations between types matter. — Den, Sep 13 '13 at 08:13

score 1 · Answer 1 · answered Sep 13 '13 at 07:52

Refactoring for me is primarily to (1) reduce code duplication and (2) make the code easier to read and understand.

Strive to have zero (or reasonably close to zero) duplicates. That's step one. Step two is to keep refactoring until you feel the code is readable enough.

For instance replacing comments with methods is often a good candidate for refactoring, like this:

// Is order valid for purchase?
if (order.age < 10 && order.customer.isActive()) {
  // ...
}

Could be refactored into:

if (isOrderValidForPurchase(order) {
  // ...
}

score 1 · Answer 2 · answered Sep 13 '13 at 08:04

Only refactor when there is an absolute need for it - refactoring is a bit of a smell, in theory you shouldn't need to refactor at all, your code should be good from the start as you designed it well... but of course, in practice this isn't always going to be the case. Nevertheless, you should be writing code that you think is good from the start, trying to achieve this impossible perfection.

If you refactor continually, you will simply shuffle the existing code around forever, never satisfied with it. You'll spend all your time refactoring and never adding any new functionality, you'll be missing the point of coding - which is not to write nice code, but to create a nice product.

The other problem with refactoring is that you can end up refactoring into the kind of code that no-one else could love, getting so caught up in the need to refactor that you screw maintainability. Your example of refactoring methods to 1 line could end up with a million tiny methods if you took it to the extreme, and worrying about how much refactoring you're doing suggests you're already going down the route of getting too caught up in it.

I have understood that the purpose of extracting things to as small methods as possible is done on purpose. That way you can detect a God Object, and you know it should be split into two or more pieces. — Juha Untinen, Sep 13 '13 at 09:16
Not necessarily, you have a God object, and afterwards you have 2 god objects. All you've done is reduce the size of a class, the code isn't necessarily better. Obviously you can improve your code using refactoring, but its not a silver bullet to fix everything, you still need to think about what you're doing and more importantly why. Blindly following some rules that tell you to reduce the size of a class is a bad thing to do, as I think you know - worrying over whether 1 class with 10 methods is as bad as 10 classes with 1 method each. — gbjbaanb, Sep 13 '13 at 09:20

score 1 · Answer 3 · edited Jun 16 '20 at 10:01

According to Martin Fowler, refactoring is

"A change made to the internal structure of software to make it easier to understand and cheaper to modify without changing its observable behavior."

Martin Fowler. Refactoring: Improving the Design of Existing Code. Boston, MA: Addison-Wesley, 2000.

This means, it is primarily up to you and your team to decide for the granularity of methods (method lenght vs. number of methods). The prime objective is to generate code that is easy to understand and cheap to modify.

Some authors tend to overplay the need for refactoring. E.g. Unkle Bob gives some examples in his book Clean Code, that are absolutely unreadable in my opinion. He's following his own guides so strictly that he does not leave room for compromises.

If three longer methods are easier to understand and cheaper to modify, stick to it.

Most of those who talk about clean code and refactoring really push the edge towards absurdly simple and small because until you've seen what code like that looks like most people assume that a 15 line function with 3 levels of abstraction is really the best you can do. — Steve Jackson, Sep 13 '13 at 11:39

Steve Jackson · Answer 4 · 2013-09-13T11:48:51.793

I think what you're struggling with is the notion of cohesion.

Cohesion is easy to define and hard to understand. It's the notion that things that belong together should be near each other.

A big part of refactoring is removing duplication. This generally leads to decoupling code, which may also reduce cohesion.

A decent analogy is the idea that cohesion is like glue, the parts really resist being pulled apart because they're integral to the structure as a whole. Coupling is like screws, it allows you to put together two very unlike items, and those items don't resist being separated (with a screwdriver).

Maintenance/Readability is really the biggest driver in this conversation - something that never EVER needs to change can be tightly intertwined in something else - who cares? But the fact that you've started on refactoring almost always means that you changed or were looking to change this code. What changes will make this easiest to change in the future? Since you can't know that in a lot of cases, the default solution is to make it as simple and understandable as possible.

One thing to think about - a one line method is often ok because you can name it much more meaningfully for your domain. If I've written a class to count the words in a phrase, being to name a method called words that splits my phrase into words is great for readability. It works especially well in ruby since methods and local variables "feel" interchangeable.

def words
  scan(/\w+/).map { |w| w.downcase }
end

Until what point should I refactor?

4 Answers4