6

I would like to refer to the example that was used before on SO with the Duck and Electric Duck:

public interface IDuck
{
    void Swim();
}

public class Duck : IDuck
{
    public void Swim()
    {
        //do something to swim
    }
}

 public class ElectricDuck : IDuck
{
    public void Swim()
    {
        if (!IsTurnedOn)
            return;

        //swim logic  
    }

    public void TurnOn()
    {
        this.IsTurnedOn = true;
    }

    public bool IsTurnedOn { get; set; }
}

The original violation for LSP would look like this:

 void MakeDuckSwim(IDuck duck)
    {
        if (duck is ElectricDuck)
            ((ElectricDuck)duck).TurnOn();
        duck.Swim();
    }

One solution by the author was to put the Logic inside the electric duck's swim method to turn itself on:

public class ElectricDuck : IDuck
{
    public void Swim()
    {
        if (!IsTurnedOn)
            TurnOn();

        //swim logic  
    }

    public void TurnOn()
    {
        this.IsTurnedOn = true;
    }

    public bool IsTurnedOn { get; set; }
}

I have come across other scenarios where an extended interface can be created that supports some sort of initialization:

public interface IInitializeRequired
{
    public void Init();
}

Electric Duck could then be extended with this interface:

 public class ElectricDuck : IDuck, IInitializeRequired
{
    public void Swim()
    {
        if (!IsTurnedOn)
            return;

        //swim logic  
    }

    public void TurnOn()
    {
        this.IsTurnedOn = true;
    }

    public bool IsTurnedOn { get; set; }

    #region IInitializeRequired Members

    public void Init()
    {
        TurnOn();
    }

    #endregion
}

EDIT: The reason for the extended interface Is based on the author saying that turning on automatically in the swim method might have other undesired results.

Then the method instead of checking and casting to a specific type can look for an extended interface instead:

void MakeDuckSwim2(IDuck duck)
    {
        var init = duck as IInitializeRequired;
        if (init != null)
        {
            init.Init();
        }

        duck.Swim();
    }

The fact that i made the initialization concept more abstract then to create an extended interface called IElectricDuck with TurnOn() method, may make this seem that I did the right thing, however the whole Init concept may only exist because of electric duck.

Is this a better way/solution or is this just an LSP violation in disguise.

Thanks

Community
  • 1
  • 1
Andre
  • 3,467
  • 4
  • 22
  • 29
  • Why not just further reduce the problem with yet another generalisation? Briefly for example: `IDuck.HasEnergy` etc, then you can enforce 'getting energy', which will be the same in principle for each duck, but different in specifics (one might eat, the other insert batteries, or turn on.) – Grant Thomas Feb 27 '12 at 12:54
  • I was thinking along these lines however the question remains that the method does not take an argument for these interfaces but rather on the highest level and then does interface casting, this could cause the same LSP violation problems when other generalizations are added for example CanGetEnergy() suddenly now all the methods would need to add this logic – Andre Feb 27 '12 at 13:01
  • What's the problem in to put the logic inside the electric duck's swim method to turn itself on? – Acaz Souza Feb 27 '12 at 13:06
  • Acaz: I think the example of Electric duck is taken too literally here, the question was the alternative to do something (else) before and posibly after swim, for instance if the method made a sql connection, should it also close it before the method ends? what if this was undesired? If so when is the connection closed? I was looking for a way to handle this – Andre Feb 27 '12 at 13:17

5 Answers5

5

It's an LSP violation in disguise. Your method accepts an IDuck, but it requries verification of the dynamic type (whether the IDuck implements IInitializeRequired or not) to work.


One possibility to fix this would be to accept the fact that some ducks require initialization and redefine the interface:

public interface IDuck 
{ 
    void Init();

    /// <summary>
    /// Swims, if the duck has been initialized or does not require initialization.
    /// </summary>
    void Swim();
} 

Another option is to accept that an uninitialized ElectricDuck is not really a duck; thus, it does not implement IDuck:

public class ElectricDuck
{  
    public void TurnOn()
    {  
        this.IsTurnedOn = true;
    }

    public bool IsTurnedOn { get; set; }  

    public IDuck GetIDuck()
    {
        if (!IsTurnedOn)
            throw new InvalidOperationException();

        return new InitializedElectricDuck();  // pass arguments to constructor if required
    }

    private class InitializedElectricDuck : IDuck
    {
        public void Swim()
        {
            // swim logic
        }
    }
}  
Heinzi
  • 151,145
  • 51
  • 326
  • 481
  • Hi Heinzi, this looks like a better approach, but how would you relate this to other forms of interface casting for instance in .NET where you own some interface references and on Disposal you to var disp = (Someinterface as IDisposable)? – Andre Feb 27 '12 at 13:26
  • @Andre: Usually, the code creating the object is responsible for disposing it (by using the `using` statement), so I guess this is quite a rare situation. – Heinzi Feb 27 '12 at 13:54
4

I would still consider your final example as an LSP violation because logically you do exactly this. As you said, there is no concept of initialization really, it is just made up as a hack.

Indeed, your MakeDuckSwim method should not know anything about any duck's specifics (whether it should be initialized first, fed with some destination after initialization, etc). It just has to make the provided duck swim!

It is hard to tell on this example (as it is not real), but looks like somewhere "upper" there is a factory or something that creates you a specific duck.

It it possible that you miss the concept of a factory here?

If there was one, then It should know what duck it is creating exactly so probably it should be responsible to know how to initialize a duck, and the rest of your code just works with IDuck without any "ifs" inside behavioral methods.

Obviously you can introduce the concept of "initialization" straight to IDuck interface. Say, a "normal" duck needs to be fed, an electrical one needs to be turned on, etc :) But it sounds a bit dodgy :)

Alexey Raga
  • 7,145
  • 1
  • 26
  • 34
  • Hi Alexey, thanks for your answer, what do you think of times where your method uses an interface but needs to also perform something additional if there is an extended interface, for example IDisposable if you need to dispose the object there is no other way then to cast and if check for disposable and do a dispose? That is the simplest example i could think of – Andre Feb 27 '12 at 13:35
  • @Andre Disposable is a different story. Disposable is _technical_, not _functional_. In other words, Disposable does nothing to do with business logic. Same for IComparable, IEquatable, etc. They do not represent behaviors from the business logic point of view. Even there, when you deal with disposables, the only thing you need is an IDisposable interface, you don't care what it is de-facto. – Alexey Raga Feb 28 '12 at 03:37
0

Maybe something like this:

public interface IDuck
{
    bool CanSwim { get; }
    void Swim();
}

public class Duck : IDuck
{
    public void Swim()
    {
        //do something to swim
    }

    public bool CanSwim { get { return true; } }
}

public class ElectricDuck : IDuck
{
    public void Swim()
    {
        //swim logic  
    }

    public void TurnOn()
    {
        this.IsTurnedOn = true;
    }

    public bool IsTurnedOn { get; set; }
    public bool CanSwim { get { return IsTurnedOn; } }
}

Client would be changed like:

void MakeDuckSwim(IDuck duck)
{
        if (duck.CanSwim)
        {
            duck.Swim();
        }
}
Zeljko
  • 140
  • 1
  • 10
0

I think first you need to answer this question about electric ducks - do they turn themselves on automatically when someone asks them to swim? If so, turn them on in the Swim method.

If not, it is the duck's client responsibility for turning it on, and you might as well just throw an InvalidOperationException if the duck can't swim because it's turned off.

zmbq
  • 35,452
  • 13
  • 80
  • 153
0
public interface ISwimBehavior
{
    void Swim();
}

public interface IDuck
{
    void ISwimBehavior { get; set; }
}

public class Duck : IDuck
{
    ISwimBehavior SwimBehavior { get { return new SwimBehavior(); }; set; }
}

public class ElectricDuck : IDuck
{
    ISwimBehavior SwimBehavior { get { return new EletricSwimBehavior(); }; set; }
}

The behaviour classes:

public class SwimBehavior: ISwimBehavior
{
    public void Swim()
    {
        //do something to swim
    }
}

public class EletricSwimBehavior: ISwimBehavior
{
    public void Swim()
    {
        if (!IsTurnedOn)
            this.TurnOn();

        //do something to swim
    }

    public void TurnOn()
    {
        this.IsTurnedOn = true;
    }

    public bool IsTurnedOn { get; set; }
}
Myles McDonnell
  • 11,575
  • 14
  • 56
  • 96
Acaz Souza
  • 7,495
  • 11
  • 49
  • 94