Illusory code duplication

Question

The usual instinct is to remove any code duplication that you see in the code. However, I found myself in a situation where the duplication is illusory.

To describe the situation in more details: I am developing a web application, and most views are basically the same - they display a list of items which the user can scroll and choose from, a second list that contains selected items, and a "Save" button to save the new list.

It seemed to me that the problem is easy. However, each and every view has its own quirks - sometimes you need to recalculate something, sometimes you must store some additional data etc. These, I solved by inserting callback hooks in the main logic code.

There are so many minute differences between the views that it is becoming less and less maintainable, because I need to provide callbacks for basically all functionality, and the main logic starts to look like a huge sequence of callback invocations. In the end I am not saving any time or code, because every view has its own code that is executed - all in callbacks.

The problems are:

• the differences are so minute that the code looks almost exactly alike in all views,
• there are so many differences that when you look at the details, to code is not a bit alike

How should I handle this situation?
Is having core logic composed entirely of callback calls a good solution?
Or should I rather duplicate the code and drop the complexity of callback-based code?

Answer 1

Ultimately you have to make a judgment call about whether to combine similar code to eliminate duplication.

There seems to be an unfortunate tendency to take principles like "Don't repeat yourself" as rules that must be followed by rote at all times. In fact, these are not universal rules but guidelines that should help you think about and develop good design.

As everything in life, you must consider the benefits versus the costs. How much duplicated code will be removed? How many times is the code repeated? How much effort will it be to write a more generic design? How much are you likely to develop the code in the future? And so on.

Without knowing your specific code, this is unclear. Perhaps there is a more elegant way to remove duplication (such as that suggested by LindaJeanne). Or, perhaps there simply isn't enough true repetition to warrant abstraction.

Insufficient attention to design is a pitfall, but also beware over-design.

Answer 2

Remember that DRY is about knowledge. It doesn't matter whether two pieces of code look similar, identical, or totally different, what matters is if the same piece of knowledge about your system can be found in both of them.

A piece of knowledge might be a fact ("the maximum allowed deviation from the intended value is 0.1%") or it might be some aspect of your process ("this queue never contains more than three items"). It's essentially any single piece of information encoded in your source code.

So when you're deciding whether something is duplication that should be removed, ask whether it's duplication of knowledge. If not, it's probably incidental duplication, and extracting it to some common place will cause problems when you later want to create a similar component where that apparently duplicated part is different.

Answer 3

Have you considered using a Strategy pattern? You would have one View class that contains the common code & routines called by several views. Children of the View class would contain the code specific to those instances. They would all use the common interface you created for the View, and thus the differences would be encapsulated & coherent.

Answer 4

What's the potential for change? For example, our application has 8 different business areas with a potential of 4 or more user types for each area. Views are customized based on the user type and the area.

Initially, this was done using the same view with a few checks here and there to determine if different things should show. Over time, some of the business areas have decided to do drastically different things. In the end, we basically migrated to one view (partial views, in the case of ASP.NET MVC) per piece of functionality per business area. Not all business areas have the same functionality, but if one wants functionality that another has, that area gets its own view. It's a lot less cumbersome for comprehension of the code, as well as for testability. E.g., making a change for one area won't cause an unwanted change for another area.

As @dan1111 mentioned, it can come down to a judgment call. In time, you may find whether it works or not.

Answer 5

One issue might be that you are providing an interface (theoretical interface, not language feature) to only a single level of the functionality:

A(a,b,c) //a,b,c are your callbacks or other dependencies

Instead of multiple levels depending on how much control is required:

//high level
A(a,b,c)
//lower
A myA(a,b)
B(myA,c)
//even lower
A myA(a)
B myB(myA,b)
C myC(myB,c)
//all the way down to you just having to write the code yourself

As far as I understood, you only expose the high level interface (A), hiding the implementation details (the other things there).

Hiding implementation details has advantages, and you just found a disadvantage - control is limited, unless you explicitly add features for every single thing that would have been possible when directly using the low level interface(s).

So, you have two options. Either you only use the low level interface, use the low level interface because the high level interface was too much work to maintain, or expose both high and low level interfaces. The only sensible option is to offer both high and low level interfaces (and everything inbetween), assuming you want to avoid redundant code.

Then when writing another one of your things, you look at all the available functionality you have written that far (countless possibilities, up to you to decide which ones might be reused) and piece them together.

Use a single object where you need little control.

Use the lowest level functionality when some weirdness needs to happen.

Its also not very black-and-white. Maybe your big high level class CAN reasonably cover all the possible use cases. Maybe the use cases are so varying that nothing but the lowest level primitive functionality suffices. Up to you to find the balance.

Answer 6

There are already other useful answers. I will add mine.

Duplication is bad because

1. it clutters the code
2. it clutters our comphrension of the code but most importantly
3. because if you change something here and you also have to change something there, you could forget/introduce bugs/.... and it is hard to never forget.

So the point is: you aren't eliminating duplication for the sake of it or because someone said it's important. You are doing it because you want to reduce bugs/problems. In your case, it appears that if you change something in a view, you probably won't need to change the exact same line in all of the other views. So you have apparent duplication, not actual duplication.

Another important point is to never rewrite from scratch something that is working now only on the basis of a matter of principle, as Joel said (you could have already heard of him....). So, if your views are working, proceed to improve step by step and don't fall prey to the "single worst strategic mistake that any software company can make".