CODEWITHSUNDEEP
c#oopinheritance
Jay
Jay

Reputation: 269

Correct to use an implementation instead of the abstraction or change implementation?

I have a situation where I have 2 Activity objects (let's say empty and scheduled activity, not controlled by me) that share a couple of behaviors, like the person who booked the activity, the room where that activity takes place, activity type, subject etc.

I created two wrappers objects (EmptyWrapper and ScheduledWrapper) that have a super class ActivityWrapper that implements some methods common to both childs and has some abstract methods/properties for the child wrappers to respond accordingly. They are very much alike in behavior but there is one crucial difference, you can only schedule activities if it is an empty slot! The structure is something like this (very simplified code):

 public class EmptyWrapper : AppWrapper
 {
    EmptySlot _emptySlot;        

    public EmptySlotWrapper(EmptySlot emptySlot) : base()
    {
        this._emptySlot = emptySlot;
    }

    public override string Id
    {
        get { return _emptySlot.AgendaId; }
    }

    public override string Room;
    {
        get{ return _emptySlot.Room;}
    }

    public override string Person
    {
        get{ return _emptySlot.Person;}
    }

    public override string AppType;
    {
        get{ return "Empty";}
    }

    public override bool IsAppSlot()
    {
        return false;
    }

    public override bool IsEmptySlot()
    {
        return true;
    }

    public override bool CanPerformOperations()
    {
        return true;
    }

    public void ReserveApp(ObjWithActivityInfo actObj)
    {
        (...)            
    }
 }

The ActivityWrapper is similar but the object wrapped around is different, the bools return true for IsAppSlot, false for IsEmptySlot and false for CanPerformOperations and there is no ReserveApp() method.

Next is the base class:

public abstract class AppWrapper
{
    public abstract string Collaborator { get; }
    public abstract string Room { get; }
    public abstract string AppType { get;}

    public AppWrapper()
    { }

    public abstract bool IsAppSlot();
    public abstract bool IsEmptySlot();

    public abstract bool CanPerformOperations();

    public virtual string GetTextToShow()
    {
        return Person + " - " + Room;
    }

    (...)
}

In my code I wanted to reference only the ActivityWrapper, because for the general operations (show the info and appearance) I don't need the implementations. The problem rises when I need to book activities for empty slots. In that point, in my code, I cast the AppointmentWrapper to the EmptyWrapper and reserve the slot for the activity (it is still an EmptySlot but it's reserved to the selected activity), otherwise, if the cast was unsucessful I don't do anything because it was not the correct Activity Type.

Is this correct, or should I implement the ReserveActivity() method in both wrappers and have the ActivityWrapper do nothing?

Or should I do this in another way? Maybe to alter the structure of the classes?

Sorry for the long text.

Upvotes: 2

Views: 142

Answers (2)

Mikey S.
Mikey S.

Reputation: 3331

In the several occasions that I had to deal with a similiar problem, I tend to think that it's really more elegant to create Interfaces for recognizing common functionailty for several objects then to create abstract methods which the inheriting classes will implement the way you mentioned.

e.g.

public interface IAppSlotContainer
{
    void relevant_Method_When_ObjectIsAppSlot();
}

public interface IEmptySlotContainer
{
    void relevant_Method_When_ObjectIsEmptySlot();
}

public class EmptyWrapper : AppWrapper, IAppSlotContainer, IEmptySlotContainer
{

public EmptyWrapper(EmptySlot emptySlot) : base()
{
    this._emptySlot = emptySlot;
}

public override string Id
{
    get { return _emptySlot.AgendaId; }
}

public void relevant_Method_When_ObjectIsEmptySlot()
{
}

public void relevant_Method_When_ObjectIsAppSlot()
{   
}

}

Then instead of overwriting the abstract method "IsEmpty" and implementing it as "return true", just check whether the object is an instance of IEmptySlotContainer, cast it to that interface, and execute the interface related command. it is far more generic and elegant for my taste...

Hope this helps...

Upvotes: 1

John Nicholas
John Nicholas

Reputation: 4836

There is no point in adding a function to a class that does not require it. It would defeat the point of your inheritance.

I'd do a safe cast ... 

var i = obj as theClass

and then test for null. I'd use a bit of linq to select all o the objects that have the property you defined to indicate what type they are set to true.

You could do it the other way and save yourself the cast and test, but it means the design is less obvious to an outsider.

I think its a matter of taste but prefer the way you did it. I am not sure i like the bool properties to identify the type though. What if you inherit off the base class again? Besides you can cast to identify the type - which with a deeper object structure may be more useful.

I agree with your desire to work with a collection of the abstract class though.

Upvotes: 1

Related Questions