views:

61

answers:

1

Hi,

I'm trying to apply OCP to a code snippet I have that in it's current state is really smelly, but I feel I'm not getting all the way to the end.

Current code:

public abstract class SomeObject
{}

public class SpecificObject1 : SomeObject
{}

public class SpecificObject2 : SomeObject
{}


// Smelly code
public class Model
{
  public void Store(SomeObject someObject)
  {
    if (someObject is SpecificObject1)
    {}
    else if (someObject is SpecificObject2)
    {}
  }
}

That is really ugly, my new approach looks like this:

// No so smelly code
public class Model
{
  public void Store(SomeObject someObject)
  {
    throw new Expception("Not allowed!");
  }

  public void Store(SpecificObject1 someObject)
  {}

  public void Store(SpecificObject2 someObject)
  {}

}

When a new SomeObject type comes along I must implement how that specific object is stored, this will break OCP cause I need to alter the Model-class.

To move the store logic to SomeObject also feels wrong cause then I will violate SRP (?), becuase in this case the SomeObject is almost like a DTO, it's resposibility it not how to know to store itself.

If a new implementation to SomeObject comes along who's store implementation is missing I will get a runtime error due to exception in Store method in Model class, it also feels like a code smell.

This is because calling code will in the form of

IEnumerable<SomeObject> sequence;

I will not know the specific types of the sequence objects.

I can't seem to grasp the OCP-concept. Anyone has any concrete examples or links that is a bit more than just some Car/Fruit example?

+1  A: 

The pattern I'm presenting attemps to register handlers for specific objects. There has to be handler registerd for every type of object that can occur. If no handler could deal with it, an exception is thrown.

You can load handlers (everything that implementes IHandler) dynamically from your or from other assemblies and instantiate and add register them. So it's enough to create a handler class for any type the implements SomeObject.

public interface IHandler {
    bool TryHandle (SomeObject o); // return true iff handled
}

public class Model
{
    private List<SIandler> _Handlers = new List<IHandlers>();

    // registers a new handler
    public void RegisterHandler (IHandler h) {
        _Handlers.Add(h);
    }

    // this tries to store an object by letting all handlers attempts to store
    public void Store (SomeObject o) {
        foreach (var h in _Handlers) {
            if (h.Store(o)) return;
        }

        // no handler was able to handle the type
        throw new Exception();
    }
}

public class Specific1Handler: IHandler
{
    public bool Handle (SomeObject o) {
        if (o is SpecificType1) {
            /* store... */
            return true; // we handled this object
        } else {
            // we're not qualified
            return false;
        }
    }
}

I believe this would fit your needs. (By the way, I don't know if this pattern has a name, would be glad to learn if there is one.)

mafutrct
This is IMHO sort of a Strategy and/or Chain of Responsibility.
Gabriel Ščerbák