views:

225

answers:

7

I have code that when given a thing it needs to sort out what specific kind of thing it is and then take special actions based on that. The possible classes it could be are all desc

public void doSomething(BaseThing genericThing) 
{
  if (genericThing instanceof SpecificThing)
  {
    SpecificThingProcessor stp = new SpecificThingProcessor((SpecificThing) genericThing);
  }
  else if (genericThing instanceof DifferentThing)
  {
    DifferentThingProcessor dtp = new DifferentThingProcessor((DifferentThing) genericThing);
  }
  else if (genericThing instanceof AnotherThing){
    AnotherThingProcessor atp = new AnotherThingProcessor((AnotherThing) genericThing);
  }
  else
  {
    throw new IllegalArgumentException("Can't handle thing!");
  }
}

Is there a pattern or better way of handling this? Unfortunately the operations being performed do not lend themselves to generalization around the BaseThing, they have to be done for each specific class of thing.

+11  A: 

The best option I can think of is to abstract the functionality in to an Interface and have each type implement that Interface.

If you add a little more detail about what you're trying to do based on the types, I could make a better suggestion (possibly with some sample code).

EDIT

After the edit, there is definitely a clear way to do this. Each Processor will implement a specific Interface:

public interface IProcessor
{
    void Process();
}

public class SpecificThingProcessor : IProcessor
{
    public void Process() { /* Implementation */ }    
}

public class DifferentThingProcessor : IProcessor
{
    public void Process() { /* Implementation */ }
}

public class AnotherThingProcessor : IProcessor
{
    public void Process() { /* Implementation */ }
}

Each BaseThing must implement a method to return the specific processor:

public abstract class BaseThing
{
    public abstract IProcessor GetProcessor();
}

public class SpecificThing : BaseThing
{
    public override IProcessor GetProcessor()
    {
        return new SpecificThingProcessor();
    }
}

public class DifferentThing : BaseThing
{
    public override IProcessor GetProcessor()
    {
        return new DifferentThingProcessor();
    }
}

And then your method will simply be:

public void doSomething(BaseThing genericThing)
{
    IProcessor processor = genericThing.GetProcessor();
    processor.Process();
}
Justin Niessner
as of now I really don't see anything in the OP's question hinting the need for an interface, more info could clear it up
eglasius
@eglasius - I defaulted to using an Interface since he said the functionality was being determined outside the scope of BaseThing. Since the OP states that BaseThing isn't the best option to handle it (and he indicates that he already thought about that), an Interface implementation was my next thought. More information would be quite nice though.
Justin Niessner
+1 Combination of Strategy pattern and Factory method pattern.
Ladislav Mrnka
There are some gotchas with this, but it looks like the right approach. If my "things" have a common basis, then my "processors" need a common basis. They already inherit from the same base processor, but not much is inherited. I think an interface will solve quite a bit of this.
Freiheit
+10  A: 

You should define a method in BaseThing to be overridden by the specific Things.

In other words you should be using a virtual function.


The operations being performed are not being performed on the generic thing. Depending on its specific type, a "Producer" class needs to be instantiated to deal with the correct type of thing. It is not appropriate to call the Producer from the BaseThing subclasses

You can still do: thing.GetProcessor(), and have each thing return the specific processor its used for it. Processors would of course implement a common interface or base class.

For another alternative, this hits my java limit, but I'm sure you should be able to do something along these lines:

  • store a list/dictionary of type, processor constructor.
  • Get the type of genericThing instance you are receiving
  • search for the type in the list and call the corresponding constructor.
eglasius
+1 The base method could be `abstract` or, if the class can't be made abstract, throw an exception like in the `else` clause in the question.
Jordão
the use of the term "virtual function" is bound to confuse some java developers.
dogbane
@fahdshariff yes, the link clears it up though, since it has a java example.
eglasius
+3  A: 

The visitor pattern is exactly what you're trying to achieve. However, a "good old-fashioned polymorphism" should do just fine for what you need. For example :

abstract class BaseThing {
    abstract public void doSomething();
}

class ThingA extends BaseThing {
    public void doSomething() {
        System.out.println("ThingA...");
    }
}

class ThingB extends BaseThing {
    public void doSomething() {
        System.out.println("ThingB...");
    }
}

class ThingC extends BaseThing {
    public void doSomething() {
        throw new UnsupportedOperationException("Cannot call this on ThingC");
    }
}

and then

class ThingHandler {
    public void doSomething(BaseThing thing) {
         try {
             thing.doSomething();
         } catch (UnsupportedOperationException e) {
             throw new IllegalArgumentException("Can't handle thing!");
         }
    }
}

Thus

ThingHandler handler = new ThingHandler();

handler.doSomething(new ThingA());  // -> ThingA...
handler.doSomething(new ThingB());  // -> ThingB...
handler.doSomething(new ThingC());  // -> IllegalArgumentException: Can't handle thing!

You have mentioned "it needs to sort out what specific kind of thing it is", so all you need now is have your BaseThing have an abstract method that will return a Comparator and each ThingA, etc. will implement it and return the proper comparator for the ThingHandler class to sort. Each BaseThing implementation can perform the specific operations or return some kind of value that you'd need in ThingHandler (you could even pass the ThingHandler instance in the BaseThing.doSomething method...)

But if the Visitor pattern is really what you need, here is an example for your need :

interface IThing {
   public void accept(ThingHandler handler);
}
interface IThingHandler {
   public void visit(ThingA a);
   public void visit(ThingB b);
   //...
}

class ThingA implements IThing {
   public void accept(IThingHandler h) {
      h.visit(this);
   }
   public String getSomeValueA() {
      return "Thing A";
   }
}

class ThingB implements IThing {
   public void accept(IThingHandler h) {
      h.visit(this);
   }
   public String getSomeValueB() {
      return "Thing B";
   }
}

// ...

class ThingHandler implements IThingHandler {
   public void visit(ThingA thing) {
       // sort according to ThingA
       System.out.println(thing.getSomeValueA() + " has visited");
       doSomething(thing);
   }
   public void visit(ThingB thing) {
       // sort according to ThingB
       System.out.println(thing.getSomeValueB() + " has visited");
       doSomething(thing);
   }

   private void doSomething(IThing thing) {
       // do whatever needs to be done here
   }
}

Then

IThingHandler handler = new ThingHandler();
new ThingA().accept(handler);  // -> Thing A has visited
new ThingB().accept(handler);  // -> Thing B has visited
//...

But since this means maintaining the IThingHandler interface every time a new IThing class is implemented, I prefer suggesting the first modified/simplified implementation of the pattern. However, feel free to adapt the pattern for your need and don't stop yourself because it doesn't exactly look like the described visitor pattern.

The two questions to ask are

  • "who is responsible to handle the operation?"
  • "who is responsible to hold the necessary data to perform the operation?"

I usually prefer keeping most of the concrete at the same place and generalize elsewhere; it helps maintaining (i.g. adding and removing features). Although the visitor pattern helps to centralize the operation in a same class...

Yanick Rochon
This is not an implementation of the visitor pattern.
Jordão
@Jordão, a pattern is a pattern, it is a gide line, not a specific rule to follow. If you really want to make this a *true* visitor pattern, simply add an argument to the BaseThing.doSomething method so it receives an ThingHandler object.
Yanick Rochon
@Yanick - no, your first is example is simply not Visitor, and isn't like it. It's just good old-fashioned polymorphism. Now, in general, polymorphism is the right solution to this problem (so your code was right, even if your description wasn't!), but if it isn't appropriate (an i tend to think that if it was, the OP would have used it already) then Visitor is a very good way of decoupling the dispatch of the call from the implementations of the different methods. Hence, upvote.
Tom Anderson
@Jordão, @Tom - Yanick's first example isn't a visitor, but the second one is a textbook Visitor implementation.
munificent
Oh yeah, now it is indeed a visitor.
Jordão
+1  A: 

This sounds like one of the basic ideas of object-oriented programming. You create a superclass that declares doSomething, and then you create subclasses each of which implements it differently. That is:

public class BaseThing
{
  abstract public void doSomething();
}
public class SpecificThing extends BaseThing
{
  public void doSomething()
  {
    System.out.println("I'm a SpecificThing!");
  }
}
public class DifferentThing extends BaseThing
{
  public void doSomething()
  {
    System.out.println("I'm a DifferentThing!");
  }
}
public class AnotherThing extends BaseThing
{
  public void doSomething()
  {
    System.out.println("I'm an AnotherThing!");
  }
}

If you really need to pass the "thing" as a parameter for some reason, okay. Do the above, then write:

void doSomething(BaseThing genericThing)
{
  genericThing.doSomething();
}

If some of your subclasses can't do the function and should give an error message instead, then just instead of making it abstrct in the supertype, make the supertype do the "invalid" processing, like:

public void BaseThing
{
  public void doSomething()
    throws IllegalArgumentException
  {
    throw new IllegalArgumentException("Can't handle this thing");
  }
}
Jay
+1  A: 

The question is almoust the text-book example of Strategy-pattern. You extract the specific behavoir into separate classes that al implement the same interface (with a method like doIt() of something). And then you give each specific class a reference to the "behavior"-object you want it to have. Bonus: 1) You can change the behavior of an object at runtime by simply given it another "behavior"-object. 2) You don't have to override a method (danger with overriding methods could be class-booming).

Cid54
+1 for good explanation of strategy pattern
nkr1pt
A: 

This could be dealt with using plain old OO polymorphism before trying to force a pattern onto it.
You don't need to necessarily subclass the processors, you can overload the method declarations in a single Processor class keeping the method name the same but declaring the parameter for the specific type.

void foo(BaseTing ting) { System.out.println("Default " + ting.name); }

void foo(TingA ting) { System.out.println("AA " + ting.name); }

void foo(TingB ting) { System.out.println("BB " + ting.name); }

Java will resolve the method that most closely matches the parameter type, so if you have TingC that extends TingB, then foo(TingB) will be invoked until foo(TingC) is defined in the Processor class.

If you are going to add a lot more actions for each type of thing, i.e. baz(Ting), bar(Ting), bat(Ting) etc. then you may want to split you Processor classes by Ting subtype and use a factory method to create the specific processor a la Strategy pattern.
i.e. BaseProcessor, TingAProcessor, TingBProcessor.
The BaseProcessor would be a good candidate to house the factory method, and should provide default implementations for each of the methods, even if the default implementation is abstract or just throws an exception. The specialised Processors classes should extend from the BaseProcessor and inherit and override the default operations.

crowne
A: 

You have few options:
* Abstract your functionality into an interface and let other classes implement that interface.
* You could use The Chain of responsibility pattern(consisting of a source of command objects and a series of processing objects)
. * You could also use the Strategy design pattern( algorithms can be selected at runtime)

walters