Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I have several classes that all inherit from a generic base class. The base class contains a collection of several objects of type T.

Each child class needs to be able to calculate interpolated values from the collection of objects, but since the child classes use different types, the calculation varies a tiny bit from class to class.

So far I have copy/pasted my code from class to class and made minor modifications to each. But now I am trying to remove the duplicated code and replace it with one generic interpolation method in my base class. However that is proving to be very difficult, and all the solutions I have thought of seem way too complex.

I am starting to think the DRY principle does not apply as much in this kind of situation, but that sounds like blasphemy. How much complexity is too much when trying to remove code duplication?

EDIT:

The best solution I can come up with goes something like this:

Base Class:

protected T GetInterpolated(int frame)
{
    var index = SortedFrames.BinarySearch(frame);
    if (index >= 0)
        return Data[index];

    index = ~index;

    if (index == 0)
        return Data[index];
    if (index >= Data.Count)
        return Data[Data.Count - 1];

    return GetInterpolatedItem(frame, Data[index - 1], Data[index]);
}

protected abstract T GetInterpolatedItem(int frame, T lower, T upper);

Child class A:

public IGpsCoordinate GetInterpolatedCoord(int frame)
{
    ReadData();
    return GetInterpolated(frame);
}

protected override IGpsCoordinate GetInterpolatedItem(int frame, IGpsCoordinate lower, IGpsCoordinate upper)
{
    double ratio = GetInterpolationRatio(frame, lower.Frame, upper.Frame);

    var x = GetInterpolatedValue(lower.X, upper.X, ratio);
    var y = GetInterpolatedValue(lower.Y, upper.Y, ratio);
    var z = GetInterpolatedValue(lower.Z, upper.Z, ratio);

    return new GpsCoordinate(frame, x, y, z);
}

Child class B:

public double GetMph(int frame)
{
    ReadData();
    return GetInterpolated(frame).MilesPerHour;
}

protected override ISpeed GetInterpolatedItem(int frame, ISpeed lower, ISpeed upper)
{
    var ratio = GetInterpolationRatio(frame, lower.Frame, upper.Frame);
    var mph = GetInterpolatedValue(lower.MilesPerHour, upper.MilesPerHour, ratio);
    return new Speed(frame, mph);
}
share|improve this question
7  
An overly micro-application of concepts like DRY and Code Reuse leads to far greater sins. –  Affe Feb 2 '12 at 21:17
1  
You're getting some good general answers. Editing to include an example function might help us determine if you're taking it too far in this particular instance. –  Karl Bielefeldt Feb 2 '12 at 22:03
    
This isn't really an answer, more of an observation: If you can't easily explain what a factored-out base class does, it might be best not to have one. Another way of looking at it is (I assume you are familiar with SOLID?) 'does any likely consumer of this functionality require Liskov substitution'? If there is no likely business case for a generalised consumer of interpolation functionality, a base class is of no value. –  Tom W Mar 29 '12 at 20:18

8 Answers 8

up vote 23 down vote accepted

In a way, you answered your own question with that remark in the last paragraph:

I am starting to think the DRY principle does not apply as much in this kind of situation, but that sounds like blasphemy.

Whenever you find some practice not really practical for solving your problem, don't try to use that practice religiously (word blasphemy is kind of a warning for this). Most practices have their whens and whys and even if they cover 99% of all the possible cases, there's still that 1% where you may need a different approach.

Specifically, with regard to DRY , I also found that sometimes it is actually better to even have several pieces of duplicated but simple code than one giant monstrosity that makes you feel sick when you look at it.

That being said, existence of these edge cases should not be used as an excuse for sloppy copy&paste coding or complete lack of reusable modules. Simply, if you have no idea how to write a both generic and readable code for some problem in some language, then it's probably less bad to have some redundancy. Think of whoever has to maintain code. Would they more easily live with redundancy or obfuscation?

A more specific advice about your particular example. You said that these calculations were similar yet slightly different. You might want to try breaking up your calculation formula to smaller subformulas and then have all your slightly different calculations call these helper functions to do the sub calculations. You'd avoid the situation where every calculation depends on some over-generalized code and you'd still have some level of reuse.

share|improve this answer
8  
Another point about similar yet slightly different is that even though they look similar in code, doesn't mean they have to be similar in "business". Depends on what it is of course, but sometimes it's a good idea to keep things separate because even though they look the same, they might be based on different business decisions/requirements. So, you might want to look at them as vastly different calculations even though they code wise might look similar. (Not a rule or anything, but just something to keep in mind when deciding if things should be combined or refactored :) –  Svish Feb 3 '12 at 11:20
    
@Svish Interesting point. Never thought about it that way. –  Phil Feb 3 '12 at 15:42

child classes use different types, the calculation varies a tiny bit from class to class.

First and foremost thing is: First clearly identify what part is changing and what part is NOT changing between the classes. Once you've identified that, your problem is solved.

Do that as the first exercise before starting the re-factoring. Everything else will fall in place automatically after that.

share|improve this answer
2  
Well put. This may just be a problem of the repeated function being too big. –  Karl Bielefeldt Feb 2 '12 at 22:00

I believe that almost all repetition of more than a couple lines of code can be factored out in some way or another, and almost always should be.

However, this refactoring is easier in some languages than others. It is quite easy in languages like LISP, Ruby, Python, Groovy, Javascript, Lua, etc. Usually not too difficult in C++ using templates. More painful in C, where the only tool may be preprocessor macros. Often painful in Java, and sometimes simply impossible, e.g. trying to write generic code to handle multiple built-in numeric types.

In more expressive languages, there is no question: refactor anything more than a couple of lines of code. With less expressive languages you have to balance the pain of the refactoring against the length and stability of the repeated code. If the repeated code is long, or liable to change frequently, I tend to refactor even if the resulting code is somewhat difficult to read.

I will accept repeated code only if it is short, stable, and refactoring is just too ugly. Basically I factor out almost all duplication unless I am writing Java.

It's impossible to give a specific recommendation for your case because you haven't posted the code, or even indicated which language you are using.

share|improve this answer
    
Lua is not an acronym. –  DeadMG Feb 3 '12 at 19:37
    
@DeadMG: noted; feel free to edit. That's why we gave you all that reputation. –  kevin cline Mar 29 '12 at 2:11

When you say the base class has to perform an algorithm, but the algorithm varies for each sub class, this sounds like a perfect candidate for The Template Pattern.

With this, the base class performs the algorithm, and when it comes the variation for each sub class, it defers to an abstract method which it is the responsibility of the sub class to implement. Think of the way and ASP.NET page defers to your code to implement Page_Load for example.

share|improve this answer

Sounds like your "one generic interpolation method" in each class is doing too much and should be made into smaller methods.

Depending on how complex your calculation is, why couldn't each "piece" of the calculation be a virtual method like this

Public Class Fraction
{
     public virtual Decimal GetNumerator(params?)
     public virtual Decimal GetDenominator(params?)
     //Some concrete method to actually compute GetNumerator / GetDenominator
}

And override the individual pieces when you need to make that "slight variation" in the calculation's logic.

(This is a very tiny and useless example of how you could add a lot of functionality while overriding small methods)

share|improve this answer

In my opinion, you are correct, in a sense, that DRY can be taken too far. If two similar pieces of code are likely to evolve in very different directions then you can cause yourself problems by trying not to repeat yourself initially.

However, you are also quite right to be wary of such blasphemous thoughts. Try very hard to think through your options before you decide to leave it alone.

Would it, for example, be better to put that repeated code in a utility class/method, rather than in the base class? See Prefer Composition over Inheritance.

share|improve this answer

DRY is a guideline to follow, not an unbreakable rule. At some point you need to decide that its not worth having X levels of inheritance and Y templates in every class you are using just to say there is no repetition in this code. A couple good questions to ask would be will it take me longer to extract these similar methods and implement them as one, then it would to search through all of them should the need to change arise or is their potential for a change to occur that would undo my work extracting these methods in the first place? Am I at the point where additional abstraction is starting to make understand where or what this code does is a challenge?

If you can answer yes to either of those questions then you have a strong case for leaving potentially duplicated code

share|improve this answer

You've gotta ask yourself the question, "why should I refactor it"? In your case when you have "similar but different" code if you make a change to one algorithm you need to make sure that you also reflect that change in the other spots. This is normally a recipe for disaster, invariably someone else will miss one spot and introduce another bug.

In this case, refactoring the algorithms into one giant one will make it too complicated, making it too difficult for future maintenance. So if you can't factor out the common stuff sensibly, a simple:

// this code is similar to class x function b

Comment will suffice. Problem solved.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.