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)

Péter Török
  • 46,547
Guillaume
  • 2,207

8 Answers8

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.

Community
  • 1
Péter Török
  • 46,547
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.

kirk.burleson
  • 1,056
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.

JacquesB
  • 61,955
  • 21
  • 135
  • 189
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?

Michael Brown
  • 21,822
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. :)

Steven Jeuris
  • 5,834
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...

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?

Darren Young
  • 2,175
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.

Andy Lester
  • 4,810