views:

728

answers:

21

My colleagues are going crazy because i keep on wanting to rewrite code that already works because i would like to replace some legacy design with design pattern. Although i feel like it will help improve the existing code, I do feel like i am getting a little paranoid about it and try to use them everywhere and even replacing one design pattern with another. some of my colleague say that so long as the legacy code works , leave it alone. When should i stop using them? where do you draw the line between code that need to be replaced by a better design and the one that needs to not be touched?

+34  A: 

If the code is working, and doesn't need attention - don't spend time/money updating it. Not until it is fiscally-necessary to do so. Just make sure all of your new code is excellent, and slowly erase this issue from now on.

Jonathan Sampson
+1: How broken is it? How much will it cost to maintain it if it does break? If it's not broken, and won't cost much to maintain, then it doesn't warrant any effort.
S.Lott
@S.Lott - I agree, "Not until it is fiscally-necessary." If it's unstable, and going to cost a fortune if neglected - fix it :)
Jonathan Sampson
@S. Lott: he didn't even say it was broken - just that it didn't match one of the Gof4 design patterns. Which is an utterly absurd reason to change code, IMHO.
MusiGenesis
+1 Seriously. Not only is this dangerous (new code is riskier than old working code), it indicates management failure: do you REALLY have nothing better to do than effectively 'jog-in-place' and do work which adds no real value?
DarkSquid
+3  A: 

Don't force a redesign - only refactor code as you need to, especially if the code already works properly. The more you change, the more you will have to test. And if there aren't existing tests, this means (in a perfect world, anyway) that you would have to write the tests.

Instead, focus on cleaning up code in small chunks, only doing what has to be done. As you go, write or update unit tests for the classes that you touch to make sure everything continues to work as it should.

Thomas Owens
+7  A: 

When you refactor then you can look at refactoring to design patterns, but there is no point in changing code just for design patterns, if it is already working.

James Black
+20  A: 

Here's a heuristic for you: the longer the code you touch has been in production without issues, the more risk you take by changing it. Given that, can you assess the true value of what you're doing? Are you making the code better? Perform better? Be more maintainable? You must quantify the benefit of the changes you are making and balance them against the risk of the refactor. Willy-nilly refactoring in general is a mistake. You should have a very good reason to do it and you should be able to quantify the benefit.

Just imagine your boss brings you into his/her office and asks you "why did you make this change when there was no problem with the code?" Will you have a good answer?

Per comment: I provide some good resources for Cost of Quality (COQ) and Cost of Non Quality (CNQ) in this SO Answer.

JP Alioto
Now, how is it that we *quantify* good code over bad code? I seem to have forgotten...
Mark Seemann
@Mark: Never doubt there is a methodology for everything. :)
JP Alioto
@Mark: simple boolean (written by me | not written by me). :)
MusiGenesis
+7  A: 

By touching the code, you create risk that once working code will stop working correctly. A better use of your time might be to make sure that the legacy code is covered by unit tests. Then when the need arises to change the code, you can refactor to whatever pattern is appropriate, and ensure that the code still works as it was designed to.

NerdFury
+1 for pointing out the risk of breaking working code
LRE
+1  A: 

Don't over-engineer when it's not needed. Took some time for me to learn this myself, though.

Ask yourself when it is needed to use a particular design pattern. When it's not the case, don't. A good reason to make use of design patterns on a legacy system is during refactoring.

petr k.
+11  A: 

Typically, Gof4 design patterns involve adding some level of indirection to make changing either side of it easier. If things aren't going to change / don't need to be made more flexible, then you don't need that indirection. If it's already there and working, no need to change. Levels of redirection are not free but have costs in terms of performance and added complexity. The GofF authors themselves pointed this out if I remember correctly.

Editing to add: OK, had to go and get the book to find the exact quote. It's at the end of the introduction on page 31, at least in the edition I have:

Design patterns should not be applied indiscriminately. Often they achieve flexibility and variability by introducing additional levels of indirection, and that can complicate a design and/or cost you some performance. A design pattern should only be applied when the flexibility it affords is actually needed.

Anon
excellent point.
alchemical
+7  A: 

I concur with Joel Spolsky that rewriting code is almost always a bad idea unless you have a very, very specific idea of what is wrong with the existing code, what rewriting it will improve, and what knowledge you are likely to lose by rewriting it.

"Rewriting code because it offends your sensibilities" is, seriously, a teribly bad idea. It's a poor use of your time, and it runs the risk of breaking something that you didn't understand.

Every line of code that you rewrite is potentially (and, in my experience, likely) a new bug that you are introducing to the codebase. Don't ever rewrite code unless you have a present and compelling reason to do so.

peterb
+1  A: 

Joel Spolsky discussed this rather intelligently on his blog.

Essentially, the older a piece of code is, the more likely that it's been thoroughly tested and debugged; even if it's full of weird fixes and things that you think are hacks, it's likely that there's a good reason for them. So avoid rewriting code no matter how much the design offends you, unless that code is definitively broken.

jprete
+7  A: 

If you were my colleague, I would be going crazy too. Design patterns are just some general ideas for how to go about solving problems. They are not the word of the Lord sent down from the mountaintop on clay tablets.

MusiGenesis
+2  A: 

for personal projects, knock yourself out.

for perfessional jobs, stop it! you are wasting resources, you could be completing requested functionality. alos, are you sure you are translating the old code 100% when you change it. if not, you are creating problems and possibly new bugs.

KM
+5  A: 

I think it's very good that you've asked the question, that's the first step towards DPW (design pattern withdrawal).

It sounds like you have the classic signs of addiction, now is the time to step back and examine your coding life, and ask things like, is this affecting my co-workers? Is my company suffering because of my habit?

Upon reflection and examination, perhaps you can find a way to moderate your design pattern use, and replace the destructive patterns with healthy ones, that bring benefits to you and your coding team.

Some helpful lines of questioning might be, do you use design patterns just because you read about a new one last night, or because you think it might add genuine value to the project? Do you force a design pattern on the code due to your strong desires, or because a real usefulness for the pattern has emerged? Does the code already work well and won't be updated anytime soon? If so, there might be better places to spend your time.

Finally, you might try experimenting with completely letting go and see what natural "patterns" emerge from the code you write, rather than trying to force the code to adapt to your ideas about how it should be. You may discover your own "patterns" this way.

Good luck, I think many of us have been there in one way or another, remember you always have your support network on SO (and common sense) to fall-back on if you have a relapse.

alchemical
A: 

I go with the "money" method. Rewriting existing code costs your employer money (your salary, etc.). Can you honestly say that your rewrite will cost your employer less than leaving it the way it is? In my experience the answer is almost always "no".

I've met more than a few whose ability to quantify cost was poor, never gave it any rational consideration, or cared more about their immediate desire to extract a paycheck than delivering good value to their employer. I personally have found good employers will come to recognize you're helping them and will come to trust your judgment over time.

Jay
A: 

Most programmers have a customer somewhere along the line, who is essentially paying the programmer's salary.

When it occurs to you to make a change like this, ask yourself, "If I made the customer understand the issues involved, would he want me to spend time on this?". Much more often than not, the answer is no. Why would they care if it works?

quillbreaker
If all coding/design decisions were made when the customer feels so, all software would be more expensive than it is today. Remember, a stitch in time saves nine. But, IMHO, customers rarely understand software well enough (even with help from programmers) to be able decide to make the stitch 'in time'. It is developers who are in a better position to understand if it needs a stitch. I do agree that sometimes, developers may be too eager to fix.
DevByDefault
A: 

if it ain't broke...dont fix it.

Nick
A: 

I disagree with everyone when you don't have a customer. Redesigning, criticizing, improving your code is what make you better. If you never want to retrospect on what you did, then you won't improve, you need to criticize yourself. Even if later you see that it was a bad idea to redesign, you'll learn why.

Nicolas Dorier
A: 

Wheneven you get the feeling

.....i keep on wanting to rewrite code that already works because i would like to replace some legacy design with design pattern...

It means Its time for you to move on to next project. What you need is a new assignment, book or an open source project to work on.

Good Luck -D

nin
A: 

Quite often I find myself using the "always leave a piece of code in a better state than what you found it" principle.

So if I load up some class and notice it could do with a bit of a spring clean before I set about doing the change request, then I will do that first. This also has another advantage in that it doubles-up for you as a way to read and understand the code and its dependencies etc.

I find that then doing the change request is much simpler because you understand the code better.

And then at the end of it all you have the best of both worlds... one happy customer and one happier code base.

NathanE
A: 

Trading different design patterns for no gain is a losing proposition. When designing an app use what you believe will be the proper design pattern and stick with it unless there are obvious reasons to change the pattern. I suggest listening to your coworkers and leaving this app alone.

RHicke
A: 

When you die. The dead are allowed to fudge it.

Cal Jacobson
A: 

I don't think "If it ain't broke, don't fix it" is right. I think you should definitely be on the lookout for code that can be refactored to make it better in a tangible way. The thing is - make sure you are doing something that will help in the future, and the part where it helps is concrete.

In design pattern terms - design patterns help you make your code more robust in the face of changes - it allows some parts of the code to vary independently of others. Think whether there is a specific reason to change something - do you anticipate future changes to some piece of code that will require touching a lot of other subsystems?

If you're clearly stating the problem you are fixing, and you are able to convince yourself and your colleagues that the problem is worth fixing (i.e. future benefit is obvious), then you should be able to move forward with it.

alps123