tags:

views:

47

answers:

3

I have a hierarchy, which I'll simplify greatly, of implementations of interface Value. Assume that I have two implementations, NumberValue, and StringValue.

There is an average operation which only makes sense for NumberValue, with the signature

NumberValue average(NumberValue numberValue){ ... }

At some point after creating such variables and using them in various collections, I need to average a collection which I know is only of type NumberValue, there are three possible ways of doing this I think:

  1. Very complicated generic signatures which preserve the type info in compile time (what I'm doing now, and results in hard to maintain code)
  2. Moving the operation to the Value level, and: throwing an unsupportedOperationException for StringValue, and casting for NumberValue.
  3. Casting at the point where I know for sure that I have a NumberValue, using slightly less complicated generics to insure this.

Does anybody have any better ideas, or a recommendation on oop best practices?

A: 

I would have create another interface IAveragable which contains the average operation which derives from Value . Then StringValue would implement just Value interface and NumberValue would implement IAveragable.

Then when it is required to use the average operation I would check if the object implements IAveragable.

tafa
That's exactly solution number two, with a new interface thrown in to complicate stuff.
Not exactly. Checking if an object implements an interface and using methods in that interface is a safe method whereas casting is not. There may be other types implementing the new interface, the collections may contain elements just implementing this new interface, so that you would not require type checking. Of course I am telling all this for future possible usages not for your specific current needs.
tafa
+1  A: 

As @tafa said, it seems to me an interface would be a good choice. Based on your signature for average, I came up with the below.

AveragableValue

public interface AveragableValue<T> extends Value
{
   public T average(T value);
}

NumberValue

public class NumberValue implements AveragableValue<NumberValue>
{
   private int _n;
   public NumberValue(int n)
   {
      this._n = n;
   }

   @Override
   public void doSomething()
   {
      // from Value interface   
   }

   @Override
   public NumberValue average(NumberValue value)
   {
      return new NumberValue((this._n + value._n) / 2);
   }
}

Then you can have your collection be of type AveragableValue. Already in your code you must have some kind of if/else clause somewhere to differentiate NumberValue and StringValue to figure out whether to call average or not. So I don't see how this would be more complicated. The hierarchy make sense - AveragableValues are a subtype of Value, and a NumberValue is a type of AveragableValue.

However, that signature for average doesn't look right. It only takes 2 values (this and the argument) and averages them. You then lose the total count of things that have been averaged before. So assuming integers as the values (as I did), something like this:

(new NumberValue(4)).average(new NumberValue(8)).average(new NumberValue(12));

would give you the value 9 instead of 8. Is this what you want? It makes it bad for many calculations done iteratively, as you may be doing with collections.

If you show us some of your code - how these classes are used, the collections holding them, how you are doing averaging right now - I can maybe give a better answer.

Phil
A: 

I'm unable to comment, therefore I'll just post a new answer.

Create an interface for value:

public interface Value<T> {
    public T getValue();
}

And one for averagable:

public interface Averagable<T> {
    public T average(T value);
}

Then a number value would be something like:

public class NumberValue implements Averagable<Number>, Value<Number>{
    public Number average(Number value) {
        // do your stuff
    }

    public Number getValue() {
        // do your stuff
    }
}

There is no need to let Averagable extend from Value.

McPudding
This is not a solution, since this typing information would get lost when looking at a collection of <Value>
then go with the solution of Phil.
McPudding