3

I have a class SuperClass with two subclasses SubClassA and SubClassB. I have a method in a different class which takes a SuperClass parameter.

The method should do different things depending on the type of the object it receives. To illustrate:

public void doStuff(SuperClass object){
    // if the object is of type SubClassA, do something.
    // if it's of type SubClassB, do something else.
}

I want to avoid type-checking (i.e. instanceof) because it doesn't feel like proper OO design.

But I can't figure out how to employ Polymorphism to elegantly solve this problem.

How can I solve this problem elegantly?

4
  • 4
    Can you post the actual (abridged) code so we can tell if it even makes sense to be using inheritance here, and if it does, if it makes sense to do type-checking? This is a bit too abstract.
    – Doval
    Aug 18, 2014 at 17:39
  • Can you be a little more concrete? The proper solution in my experience is to eliminate the "method that takes a SuperClass" altogether by designing your solution a different way.
    – Telastyn
    Aug 18, 2014 at 17:39
  • I would say this method is poorly designed from the outset. A method should do one thing and that one thing well. This already has the method doing 2 things. If they're expected to do 2 different things they should be 2 different methods with type specific parameters. Since type is the criteria for behavior you're already "tightly coupled" at least logically. Aug 18, 2014 at 20:48
  • you can avoid type checking and end up handling the ClassCastExceptions...
    – jwenting
    Aug 20, 2014 at 12:21

4 Answers 4

9

Basically, doStuff should belong to SuperClass instead of your other class, then SubClassA and SubClassB each have their own implementations. Sometimes you might be able to pull only a small part of doStuff into SuperClass. You might need to make OtherClass an argument of doStuff.

It's hard to say more without more details. The bottom line is you need to try moving functionality to different classes until you find an arrangement that isn't so awkward.

4

The problem here is that either doStuff or SuperClass (or both) is too general to accurately describe what should be done in this action (method). The only reason you would need to check the types here is that they are poorly designed to begin with and don't offer enough of a contract to take that action.

Since you don't say what doStuff or SuperClass is actually referring to, let's say it's log(LogEventBase logEvent). Looking at this method signature, there are gaps of information that need to be filled.

  • What is actually being written to the log from the logEvent?
  • Where are we logging to?
  • What does it mean to be a LogMessageBase?

There are a number of other potential unanswered questions here, but you might think the solution is to base the method on the actual Type of LogEventBase:

public void log(LogMessageBase logEvent)
{
    switch (logEvent.GetType()):
        case BusinessLogicLogEvent:
            logToUserInterface(logEvent);
        case ErrorLogEvent:
            logToErrorLogs(logEvent);
        ...

This is bad!

If it was properly designed, the answer to those questions would be written into the Types/methods directly in a number of different ways. An example of one of those ways is with a better defined interface:

public ILogMessage
{
    public GUID Id;
    public LogLevel Level;
    public string Message;
}

public void log(ILogMessage message)
{
    switch (message.LogLevel):
        case LogLevel.Error:
            logToFile(...

// or

public void logToConsole(ILogMessage message)
{
    Console.WriteLine(string.format("Message: {0}. Id: {1}",
        message.Message,
        message.Id);
}

The point being that the Interface describes a contract for what it means to be an ILogMessage, not the Type, which could quickly grow and become unmaintainable.

There isn't a canned answer for your particular problem without knowing the data types involved. In fact, putting LogLevel directly on the ILogMessage interface could be completely wrong for your use case. I certainly don't mean to imply that you should specify an Interface that has a TypeName property and shift the responsibility there. That particular solution was only there to illustrate an example of fixing the problem (that LogMessageBase was too broad of an argument for this method) at the source so you don't need to switch on type to begin with.

4
  • This is bad! It's only bad when you expect to add more classes of logEvents. There are types for which the number of classes of its values is fixed (e.g. it's rare to need 2D point representations other than polar and rectangular.)
    – Doval
    Aug 18, 2014 at 18:08
  • I disagree. That was only one example of why the code could become tricky to maintain. Once you start splitting functionality on Type in multiple ways, you completely lose the ability to let the Types/Interfaces speak for themselves and you end up leaving consumers to guess at how they are used because no contract exists.
    – Ocelot20
    Aug 18, 2014 at 18:37
  • 1
    There's a trade-off between both approaches. When you commit to a fixed number of cases, it's easy to write new functions but hard to add new cases (the new case needs to be added to all existing functions). When you commit to an unbounded number of cases (i.e. open inheritance) it's easy to add new cases (add a new subclass) but hard to add new functions (all existing subclasses need to be fixed). To say one is better than the other is wrong, and you can't pick one without knowing what the OP is trying to do.
    – Doval
    Aug 18, 2014 at 18:44
  • Sure, there are always trade-offs to consider (is this a one-off script vs. a enterprise application? etc.). The question/context so far is based on how to do this in an OOP appropriate way, which I still submit is not the right way to go about it if you're trying to stick to the tenets of OOP. Your last line could be argued for just about anything :). Hopefully OP chimes in with something a bit more concrete.
    – Ocelot20
    Aug 18, 2014 at 18:59
2

I think you can use the visitor pattern here. The class with the doStuff method should implement the Visitor interface. This interface has overloaded doStuff(..) (or visit) methods for all subtypes of the SuperClass. The SuperClass has an abstract method accept(Visitor) method and all subclasses implement this by visitor.doStuff(this), calling the correct method on the visitor.

This pattern is a fix for the static binding that e.g. Java has: method calls are only dynamic with respect to the type of object they are called on, not the parameter types.

1

I would agree with Karl Bielefeldt's answer to move doStuff to the SuperClass with different implementations in SubA and SubB, or more specifically keep doStuff where you have it and only call methods on the passed in SuperClass object in the places where the the implementation differs for SubA and SubB:

public void doStuff(SuperClass object){
    // do common stuff here

    object.specificStuff(<whatever parameters>);

    // do more common stuff here

    int bar = object.moreSpecificStuff(<some parameters>);

    // room for still more common stuff, perhabs based on bar's value 
}

Then Superclass would have abstract (or pure virtual) methods:

class SuperClass {

   ( ... )

   virtual void specificStuff( <whatever parameters> ) = 0;  // assuming C++

   virtual int moreSpecificStuff(<some parameters>) = 0;  // assuming C++ 

};

and SubA and SubB would provide implementations for those.

This is one way to implement the strategy pattern (from the Design Patterns book, Go4).

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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