Add more behaviour without creating new classes
Asked Answered
T

6

11

This was the question asked in an interview.

There is a Label with a property Text
In one page a label is simple Label, in other pages it may handle any one or combination of the below actions
Clickable
Resizable
Draggable

How do you design this label component that applies OOP design Principle & Design Pattern?

I said that I would create the following:

public class Label
{
  public string Text{get;set;}
}
public interface IClickable
{
 void Click();
}

public interface IDraggable
{
 void Drag();
}
public interface IResizable
{
 void Resize();
}

So that if the client want Resizable Label

public class ResizableLabel:Label,IResizable
{
  ....
}

same way ClickableLable, DraggableLabel

However, I feel that this is the incorrect approach, because I do not want to add those concrete classes. I want to avoid having ClickableAndDraggableLabel or ClickableDraggableResizableLabel.

Is there any design pattern that would solve this problem without adding these concrete classes?

Titania answered 19/4, 2013 at 7:40 Comment(3)
"Without creating new classes"... Isn't ResizableLabel in your example a new class then?Scoundrel
Did you ask the interviewer what was correct? What was his/her response to your solution?Mcleod
@MatthewWatson, I would like to refactor my code. See the answer from Ilya. It looks goodTitania
L
9

I would use Decorator pattern. It is used extensivelly in .net world for different kind of streams, that allow you to compose encrypted, zipped, text stream wrappers for byte stream, for example. class diagram is taken from wiki

enter image description here

Example for you situation is not so trivial in implementation, but usage doen't require another classes for new compising behavior:

// Define other methods and classes here
public class Label
{
    public string Text{get;set;}

    public virtual void MouseOver(object sender, EventArgs args) { /*some logic*/ }
    public virtual void Click(object sender, EventArgs args) {  /*some logic*/ }

    //other low level events
}

public class ClikableLabel : Label
{
    private Label _label;

    public ClikableLabel(Label label)
    {
        _label = label; 
    }

    public override void Click(object sender, EventArgs args) 
    {   
        //specific logic
        _label.Click(sender, args);
    }
}

public class DraggableLabel : Label
{
    private Label _label;

    public DraggableLabel(Label label)
    {
        _label = label; 
    }

    public override void Click(object sender, EventArgs args) 
    {   
        //specific logic
        _label.Click(sender, args);
    }
}
public class ResizableLabel : Label
{
    private Label _label;

    public ResizableLabel(Label label)
    {
        _label = label; 
    }

    public override void MouseOver(object sender, EventArgs args) 
    {   
        //specific logic
        _label.MouseOver(sender, args);
    }

    public override  void Click(object sender, EventArgs args) 
    {
        //specific logic
        _label.Click(sender, args);
    }
}

now you can

var clickableDragableLabel = new ClikableLabel(new DraggableLabel(new Label{Text = "write me!"}));

var makeItResizable = new ResizableLabel(clickableDragableLabel);
Lyndel answered 19/4, 2013 at 7:51 Comment(9)
Why this label class needs to abstract? In some places they just want to define simple label which is not clickable, draggable,etcTitania
Don't do this. ResizableLabel should not known about Clickable. It doesn't make sense that ResizableLabel implement a Click method. Decorator pattern is not meant to do this.Terresaterrestrial
How would this work if clickable/clickdrag/dragable textboxes all need their text to be a different colour for example?Ophthalmoscopy
I like this approach but @Billa, you asked to do this without adding more concrete classes?Trucker
@Trucker i meant with out adding a new class 'ClickableResizableLabel` to achieve click and resize because we already have those classes seperatelyTitania
Oh I see. Then I think this solution is good. @Scorpi0 is right, Resizable should not know about Clickable but, could you not change this to pass an interface type to the constructor of each, that is inherited by all. Like for example ILabel?Trucker
The reason I don't like this approach is it seems like it would have multiple properties that will be duplicated everytime you further nest a label, interesting to see though, (such as multiple onPaint events too!)Ophthalmoscopy
What happens if you have 20 new attributes you want to apply? What's the behaviour if I create new ResizableLabel(new ResizableLabel(new ResizableLabel()))? Does it matter if Clickable or Resizable is the outer class? This method poses a lot of questions, the answers to which would not be obvious to the consumer, and for that reason I don't like it. Also, because your behavioural classes all derive from Label, you've lost the ability to apply them to, say, a Button later.Locklin
@Scorpi0 ResizableLabel should not known about Clickable it simply doesn't, it depends only on Label. @DanPuzey What happens if you have 20 new attributes you want to apply? - it depends on what level you want to add such behavior. you've lost the ability to apply them to, say, a Button no you don't. Create a common abstraction between button and a label and make decorators to them. Guys, I agree that this is not the best approach at all (I doubt there is), but given the OPs concern Add more behaviour without creating new classes - it at least address it in some way.Lyndel
O
1

I would just have boolean properties for CanClick, drag, and resize, all default to true, and falsed as required (or as inherited).

constructor as follows

public Label(bool canClick = true, bool canDrag = true, bool canResize = true){}

Chances are if they're extending a class once, its going to be extended further at a later date

Ophthalmoscopy answered 19/4, 2013 at 7:44 Comment(0)
T
1

I don't think Interface can resolve your problem.
I would make something more like this:

First, define an enum which list all your action:

public Enum LabelAction{ None = 0, Clickable = 1, Resizable = 2, Draggable = 4 }

For having multiple Enum defined, you can look this links:

Then define a member in your class Label, taking an action:

public class Label
{
    private readonly LabelAction _action;
    private string Text { get; set; }

    public class Label(string text)
        : Label(text, LabelAction.None) { } 

    public class Label(string text, LabelAction action)
    {
        this.Text = text;
        this._action = action; 
    }

    public bool CanClick 
    { 
        get
        {
            return this._action & LabelAction.Clickable == LabelAction.Clickable;
        }
    }

    public bool CanResize { get { return this._action & LabelAction.Resizable == LabelAction.Resizable ;} }
    public bool CanDrag { get { return this._action & LabelAction.Draggable == LabelAction.Draggable ;} }

    public Click()
    {
       if(this.CanClick) { /* click */ }
       else { throw new Exception("Not clickable");}
    }
    public Drag()
    {
       if(this.CanDrag) { /* drag */ }
       else { throw new Exception("Not draggable");}
    }
    public Resize()
    {
       if(this.CanResize) { /* resize */}
       else { throw new Exception("Not resizable");}
    }
}

Usage:

var simpleLabel = new Label("simple");
var clickable = new Label("clickable", LabelAction.Clickable);
var clickableDraggable = new Label("clickable and draggable", LabelAction.Clickable | LabelAction.Draggable);

public void DoEvent(Label label)
{
    if(label.CanClick) label.Click();
    if(label.CanDrag) label.Drag();
    if(label.CanResize) label.Resize();
}

If you need to add an action, you will have to add one item to the Enum LabelAction, one method CanDo() and one method Do() to the Label class. Not so much so.

Terresaterrestrial answered 19/4, 2013 at 7:52 Comment(0)
F
0

Well you can have a base class that implement all the interface and delegate their behaviour to concretes strategy classes. Then you would have a NullDraggable, nulResizeable,NullClickable by default that do nothing (so your base label is not clickable, resizable and dragrable) Then you create different strategy, like Clickable, DoubleClickable, WidthResizeable etc... You then pass the combination you want to your class. This way you obtain a lot of little strategy that are easy to reuse in other component with the same interface. You can have multiple behaviour by using a composite pattern (for example you can have clickable and doubleclickable togheter)

This probably would be a little too ingeneered though

Fishmonger answered 19/4, 2013 at 7:45 Comment(0)
C
0

I think you're over-thinking what the interviewer must have had in his mind. If the case is as simple and practical, to avoid the complexity of over abstraction, then this would suffice:

public class Label
{
    public string Text{get;set;}
}

public class ComlexLabel : Label
{
    Click();
    Drag();
    Resize();
}

You can do any operation on it. Now if for a challenge you need only one concrete instance and need separate type of objects to be able to do only a combination of these things, its again simple - only that this time you have to create similar prototypes/interfaces:

public class Label
{
    public string Text{get;set;}
}

public interface Clickable
{
    Click();
}

public interface Resizable
{
    Resize();
}

public interface Dragable
{
    Drag();
}

public interface ClickableDragable : Clickable, Draggable
{

}

public interface ClickableResizable : Clickable, Resizable
{

}

public interface ResizableDragable : Resizable, Draggable
{

}

public interface ClickableDragableResizeable : Resizable, Clickable, Draggable
{

}

public class ComlexLabel : Lable, ClickableDragableResizeable
{
    Click();
    Drag();
    Resize();
}

Now you can have instances of ComlexLabel by making the type that gives the required feature. Like:

ResizableDragable rd = new ComlexLabel();
ClickableResizable cr = new ComlexLabel();
ClickableDragableResizeable cdr = new ComlexLabel();

Now rd, cr and cdr have different capabilities. And only one concrete instance behind them. To prevent the clients from getting full privilege by doing

var cdr = new ComplexLabel();

you should make the ComplexLabel constructor private and assign the task to some factory. Like

var rd = Factory.GetResizableDragableLabel();

Now rd must be just ResizableDragable with no Click functionality..

Cusco answered 19/4, 2013 at 8:15 Comment(2)
This code is not maintainable. Need to add as many classes, if i add Movable functionality tomorrow :(Titania
@Titania no you are adding only interface/contracts. But yes, combinations become problematic. But then I think interviewer was testing with a not so real world example. Just being little rigid about your "only one concrete class" demandCusco
L
0

I think for this scenario, there is no need to re-invent the wheel. Even though the question explicitly asks for OOP it is not explicitly asking you to ignore Component Model programming nor event based behaviors. That's why I would follow an approach that allows a division of responsibilities where Label is responsible to notify when it is being clicked or dragged and SomeOtherComponent might or might not listen to such notification (event) in order to perform other logic.

Please take a look at the links below for examples of the approach of event dispatching for those user actions :

Drag and Drop

Label Class

Regards,

Lair answered 19/4, 2013 at 8:52 Comment(1)
Hello, May I ask why the down-vote. I would like to understand which is the mistake in the answer :) Best Regards,Lair

© 2022 - 2024 — McMap. All rights reserved.