46

I have to extend an existing module of a project. I don't like the way it has been done (lots of anti-pattern involved, like copy/pasted code). I don't want to perform a complete refactor for many reasons.

Should I:

  • create new methods using existing convention, even if I feel it wrong, to avoid confusion for the next maintainer and being consistent with the code base?

or

  • try to use what I feel better even if it is introducing another pattern in the code ?

Precison edited after first answers:

The existing code is not a mess. It is easy to follow and understand. BUT it is introducing lots of boilerplate code that can be avoided with good design (resulting code might become harder to follow then). In my current case it's a good old JDBC (spring template inboard) DAO module, but I have already encounter this dilemma and I'm seeking for other dev feedback.

I don't want to refactor because I don't have time. And even with time it will be hard to justify that a whole perfectly working module needs refactoring. Refactoring cost will be heavier than its benefits. Remember: code is not messy or over-complex. I can not extract few methods there and introduce an abstract class here. It is more a flaw in the design (result of extreme 'Keep It Stupid Simple' I think)

So the question can also be asked like that:
You, as developer, do you prefer to maintain easy stupid boring code OR to have some helpers that will do the stupid boring code at your place ?

Downside of the last possibility being that you'll have to learn some stuff and maybe you will have to maintain the easy stupid boring code too until a full refactoring is done)

Improve this question
5
  • Very interesting question!
    – Philippe
    Feb 11, 2011 at 14:23
  • 1
    Maintaining easy boring stupid code - is by definition easy. I'd rather have an easy life and leave early each day.
    – quickly_now
    Feb 13, 2011 at 10:07
  • Yeah, but I'm too lazy to do boring stupid code :) I'll prefer work hard for 2 or 3 days to build something that will do this part for me !
    – Guillaume
    Feb 15, 2011 at 8:51
  • 1
    When did stupid or boring get confused for easy?
    – Erik Reppen
    Jan 18, 2014 at 4:45
  • Hi Erik, sometimes a code that could have been factorized is more simpler to read: it is dozen of time the same thing and the code is straightforward. You can read it in a row without to think about it and you can debug in a linear way. There is no hidden complexity.
    – Guillaume
    Jan 26, 2014 at 13:52

8 Answers 8

Reset to default
43

Refactoring is best done in small steps, and preferably only if you have unit tests to cover the code. (So if you don't have tests yet, strive to write them first, and until then, stick to the simplest, most foolproof, preferably automated refactorings. A great help in this is Working Effectively with Legacy Code by Michael Feathers.)

In general, aim to improve the code a little whenever you touch it. Follow the Boy Scout Rule (coined by Robert C. Martin) by leaving the code cleaner than you found it. When you add new code, try to keep it separated from the existing bad code. E.g. don't bury it into the middle of a long method, instead add a call to a separate method and put your new code in there. This way, you grow gradually bigger islands of clean(er) code within the existing codebase.

Update

Refactoring cost will be heavier than its benefits. [...] You, as developer, do you prefer to maintain easy stupid boring code OR to have some helpers that will do the stupid boring code at your place ?

I emphasized which I believe is the key point here. It is always worth assessing the costs and benefits of refactoring before we jump into it. As in your case, most of us have limited resources for refactoring so we must use them wisely. Spend that precious little time on refactoring where it brings the most benefits with the least effort.

As a creative mind, of course I would prefer producing perfect, beautiful and elegant code, and rewriting everything which does not resemble my ideals :-) In reality though, I am paid to produce software which solves real problems for its users, so I should think about producing the most value for their money over the long term.

The benefit of refactoring only appears if there is sufficient savings in time and efforts to understand, maintain, fix and extend the code in the long term. So if a piece of code - however ugly it is - is rarely or never touched, there are no known bugs in it and I don't know of any upcoming features in the foreseeable future which would require me to touch it, I prefer leaving it in peace.

Improve this answer
7
  • 8
    It's so easy to fall into the trap of 'the code already sucks, may as well keep it going...' Good programmers don't. Nice answer.
    – sevenseacat
    Feb 11, 2011 at 13:03
  • Although that's most likely the safest approach (code is guaranteed to work thanks to the tests) it does still result in the problem mentioned in the question. A different pattern is introduced into the code. Generally, I believe you should try to take your time to refactor everything at once (when doing TDD, best with intermediate steps) so you don't have that intermediate inconsistent state. When finished, commit your code to source control.
    – Steven Jeuris
    Feb 11, 2011 at 13:30
  • 2
    @Steven, it is fine to refactor everything at once if you can afford it. In most real life projects though, you can't just stop everything and spend several days or weeks in a row only to refactor. Even if you are lucky to have a manager who supports it, the project still needs to roll on.
    – Péter Török
    Feb 11, 2011 at 14:16
  • 1
    @Steven, it depends - see my update above.
    – Péter Török
    Feb 11, 2011 at 14:38
  • 1
    @Péter Török: Nice update! Perfect description of real world factors to take into account.
    – Steven Jeuris
    Feb 11, 2011 at 14:42
4

Since you don't have time to refactor and the code is maintainable, keep it consistent. The next time around be sure to include refactoring in the estimate.

Improve this answer
3

Being consistent only aids the next developer when the surrounding code is in good shape. Think of how long it took you to understand the code at hand and to find the right "extension points" to add your changes. If you follow the current trend, then the next guy has to understand the existing code as well as your new code when he has to make changes.

If you refactor the code into meaningful functions, the next guy will have an easier time comprehending what is happening and will need to take less effort to add his changes. Think of it this way. Assume the next guy is you. What would you prefer to see when you revisit this block of code, the same mess you're looking at now, or something more logically constructed?

Improve this answer
3

Consistency has a high priority. Obviously everyone in their right mind would prefer an elegant DRY solution everywhere instead of copy-pasted boilerplate everywhere, but would you really prefer two different approaches in the same codebase to having one consistently applied? What if someone invents an even smarter solution and also applies it inconsistently? Then you have three different ways of doing the same thing.

I have seen much messy code resulting from developers finding a "better way" to do something, but not applying it consistently over a codebase. If it is part of a planned and coordinated strategy to apply a new pattern gradually over time it may work, but every pattern implemented inconsistently have the risk of making the overall system worse.

Maybe you could consider refactoring in smaller steps, such that each step can be applied over the whole codebase in one go. Eg. extracting a smaller part of the boilerplate to a helper function in each iteration. Over time you end up with all the boilerplate in a helper class. It may look really ugly because it wasn't designed but grown, but you can fix that now because you have all the code in one place.

Improve this answer
1
  • +1 "Then you have three different ways of doing the same thing." I have seen this in every enterprise project I've worked on. I'd say don't even try to refactor until you've scoured the code to be sure there isn't already another piece of code that is attempting to take care of the ugly code that you decided you want to refactor. Sometimes it is best to just stick with boilerplate convention, at least until you've given the code a thorough read.
    – TK-421
    Aug 18, 2015 at 15:26
1

Any refactoring which eliminates duplicate code is good refactoring, and shouldn't be postponed unless a deadline is imminent. The time spent on refactoring once, is easily made up for by time won in future implementations. Thanks to the refactoring the inner workings aren't visible anymore, so it should become easier to understand, not harder to follow.

In my opinion it's a common misconception that refactored code is harder to follow. This is usually an argument of people who are only familiar with what is being refactored, and not the refactored result. For newcomers, the refactored result will be clearer.

Any bugs/features can be fixed/implemented in a central place at a later time, and are automatically available everywhere in your application. This is a huge gain which is usually highly underestimated. In general a total refactoring of a component doesn't take that long. (days instead of months) When comparing this to time lost due to bugs, having to understand code which could have been refactored, the cost of refactoring becomes minimal.

Update relating to Péter Török's reply: Isn't it by postponing refactoring "because it takes time", the time to refactor it at a later time increases? Refactoring shouldn't take long, when done more often.

How to explain to your boss that you will spend a few days writing code which doesn't add anything to the product, is difficult, and is an entirely different question. :)

Improve this answer
7
  • 1
    "How to explain to your boss that you will spend a few days writing code which doesn't add anything to the product, is difficult, and is an entirely different question." Nice try ;-) Unfortunately in real life we need to solve this too. That's why it is more practical to go in small steps. Half an hour of refactoring can be done as part of a two-hour task, without your manager even needing to know about it. OTOH two full days spent refactoring rarely goes unnoticed.
    – Péter Török
    Feb 11, 2011 at 14:22
  • @Péter Török: I updated the post a bit. Of course you still have real world situations where you aren't allowed to take the time for refactoring. But I strongly believe that in theory it's always time gained. (unless you know the code you are working on won't be supported anymore in the near future)
    – Steven Jeuris
    Feb 11, 2011 at 14:26
  • as they say, in theory there isn't much difference between practice and theory. However, in practice... :-) In real life you may not always get back the cost spent on refactoring. I extended my answer on this.
    – Péter Török
    Feb 11, 2011 at 14:41
  • 1
    Refactoring which removes duplicated code is not always good; it's possible that two methods might be identical under today's business rules, but that not tomorrow's. For example, an AcmeLockerBank one might have a getRow(int itemIndex) method which returns index % 5 and an AcmeButtonPanel might have an identical method because both the locker and button boards number things the same way; if none of today's locker banks have items with an index over 1000 but some button panels do, and future lockers use a different row spacing for indices in the 1000-1999 range...
    – supercat
    May 18, 2015 at 19:30
  • 1
    ...adding the additional behavior to locker banks will be easy if locker banks and button banks have distinct getRow methods, but may be more difficult if the functions have been merged to a common method.
    – supercat
    May 18, 2015 at 19:32
0

Can you not create a new Interface/Classes that works as a Facade over the old code, whilst introducing your new code in your own style that can be easily extended?

Improve this answer
0

Do it right going forward. Use functions instead of cutting and pasting. That doesn't require refactoring, but it gives you the start of a foundation for future refactoring.

Improve this answer
0

You, as developer, do you prefer to maintain easy stupid boring code OR to have some helpers that will do the stupid boring code at your place ?

I don't understand the revised question. I think most people, as myself, would prefer not to maintain "stupid boring code". I don't know what you mean by helper, sorry.

The poster answered their own question by not making big changes right now. Good choice. I have problems with a developer's arrogance that they know what's best for the business. A big refactoring on the sly... because you know better than your boss? Any capable manager can weigh up benefits.

If refactoring isn't an option then the question becomes more of a discussion about the correct time, place, etc... for doing it. Consistency is very important. Yet long lived code often benefits from "renewal". Others have discussed this...

Improve this answer

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.