views:

85

answers:

4

We're using a code static analysis tool (Sonar) to detect duplicate code on our project. It appears that there are a great many instances of duplicates in the code but most of them are less than 10 lines and occur only once. In the 'opinion' of the Stackoverflow community, Where should you draw the line at consolidating code duplications? For example should it be at >= 10 lines for individual duplicates or should you consider overall number of lines duplicated eg. consider > 10 duplicates that occur more than once. For context the programming languages I'm looking at are Java and ActionScript. I realize that this question may not come to a definitive answer but some clear guidance on this matter could save me a lot of time refactoring code (or increase the time:)

+4  A: 

If they are actual duplicates, consolidate as soon as you can. The longer you leave it, the more the copies will diverge and the harder it will be. Also, when someone comes across a pile of copies and needs to do a "quick fix" they will make more copies, but a nice clean codebase encourages everyone to keep it clean.

Matt Curtis
+2  A: 

Ideally: Consolidate now.

More practically: For smaller segments like yours a good test is to check if you define a meaningful purpose for the code you've found. If the code's purpose can be summed up in a few words, you probably can move it to a separate function. If you can't sum it up but it's still duplicated, I'd say leave it.

Ideally your code should not allow this, but in most code multiple things end up going into one subroutine - for example the code to open a socket and send a packet. Ideally these are different things, therefore they are different functions. If your production code isn't this perfect, you could end up with the end of the socket code and the start of the packet code being the same (about 10 lines worth) before changing later on. If these match, but you can't describe its function in a few words, leave it and re-factor the rest of your code when you have time.

Nathan
+2  A: 

Consolidate code (especially small blocks like the 10 lines you mention) only if it has a common purpose. If the code snippets are really doing different a task (but the implementation happens to be the same), then there's not really any benefit from a design point of view of consolidating them, and you may have to split them out again later if you do.

davmac
+2  A: 

Consolidate code, if it is meant to do the same and if you want all changes applied to one duplicate to be applied to the other duplicate. You may not always want that to happen. You may have two pieces of code, doing virtually the same thing, but fullfilling completely different purposes, in order to satisfy different requirements. Requirements fluctuate, and soon you may find, that the section you have unified cannot keep up with shifts occuring in different directions (a situation I ran into quite often, when overdoing this).

So the rule is simple: Consolidate code, that does the same thing. No matter how long. You don't want to make the same change in a 1000 places. Do not try to consolidate code sections, which incidentally appear similar unless you find they have enough common ground, that could be refactored into a useful subroutine. Also keep in mind, performance critical code is not the best place for best practices.

greetz
back2dos

back2dos
Also keep in mind, performance critical code is not the best place for best practices.Beware of this kind of reasonning. Often time "premature optimization is the root af all evil" and performance critical code only has to be ugly if its a proven benefit :)
Axelle Ziegler