Once and only once - with more lines of code

Question

I have an ugly bit of code - essentially iteration over some data structures where the meat of the action was changing, but the iteration code stayed same. The iteration constituted the bulk of code, and there were at least four cases of code copying.

So I refactored the code, putting each level of iteration into its own class (I'll probably do another post on it later).The new code is considerably nicer (at least to my eye), with no copy/paste locations. However, due to some infrastructure I had to introduce to refactor nested loops into separate classes, I ended up with 10% larger number of LOCs. If I discount the extra infrastructure, I get about 5% smaller code compared to the original size.

So, the code is more sophisticated, but it is not shorter and may be harder to understand for a less experinced programmer. I may still end up with shorter code in the future if more oppotunities for reuse present themselves.

My question is: do you think it was worth it? Is it a good idea to refactor for "once and only once" if the LOC count goes up?

Answer 1

Lines of code is not a good measure of code complexity. It should only be used as a rough estimate of project size. For example, a 1k LOC program is generally simpler than a 10k LOC program. I'm assuming that that actual number of lines you are talking about stays within the negligible spectrum.

Reducing code repetition is always good because it prevents mistakes later if you need to change something.

Answer 2

Hard to say without seeing the code you refactored, but removing code duplication is always a good thing in my mind, even if it ends up increasing the LOC count.

Answer 3

Coding style is about communicating intent to people, whether yourself in six months time, or your co-worker on the project, or whoever has your job after your move on.

So if your code is clearer and more understandable, that's a big win. There will be fewer bugs in it, and fewer bugs in code that interacts with it.

I think your point about potential later reuse is a good one as well.

Answer 4

Repeated code is bad because it's easy for the copies to go out of sync when you have to make a change to how they function. Even if you don't forget to update all the copies when you change one, you're still going to need to make four( or however many copies you have) changes every time you make a change.

And at that point you're likely to say "four copies of the same code? I should refactor this."

Answer 5

Slightly longer code can be easier to maintain if it means that a policy change can be implemented in a single place. Collecting a complex nested iteration into a class is a good idea, but in my experience most programmers are not familiar with this pattern.

Answer 6

Following DRY (“Don’t Repeat Yourself”) has been worth the effort in my experience. I know when I work on my code that things are specified once and only once, and that simplifies understanding and developing the code.

Functional languages can help with this because you can pass a function into a method that applies the function to every element of a collection, for example.

I was screening a code sample from a job applicant for a client recently and I saw two methods that were identical except for the method names, and the names of one of the identifiers. I knew I needed to look no further at the applicant.

Answer 7

As you describe your problem, I don't understand why each level of the iteration needed to go in a separate class. It sounds like you have something like nested collections, and you need to consider all elements in some certain order, but multiple times. That is, you need one procedure, which takes the top level data structure, and and an operation to perform on each element, and then applies the operation on each element in the prescribed order. If you've changed using bare data types into calling into wrapper classes, but your code still repeats nesting, you haven't gained anything.