HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Subclassing, NVI

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
nvisubclassingstackoverflow

Problem

Should the following be considered wtf code?

abstract class BaseCat
{
    public virtual void SayMiau()
    {
        Console.WriteLine("MIAU!");
    }
}

class StrangeCat : BaseCat
{
    private readonly bool _canMiau;

    public StrangeCat(bool canMiau)
    {
        _canMiau = canMiau;
    }

    public override void SayMiau()
    {
        if (_canMiau)
        {
            base.SayMiau();
        }
    }
}


What's the right approach here? Non-virtual interfaces (NVI)?

Solution

The example you show has no rationale for the BaseCat being abstract, make it non-abstract and your example makes perfect sense. Though I would make a strong case that your BaseCat may want to implement the structure of StrangeCat. Then have StrangeCat merely implement a change to the _canMiau if valid.

public class BaseCat
{
    protected bool _canMiau;

    // if you wanted abstract so BaseCat could not be constructed make the constructor protected
    protected BaseCat(bool canMiau) { _canMiau = canMiau; }

    public virtual void SayMiau()
    {
        if (_canMiau)
        {
            Console.WriteLine("MIAU!");
        }
    }
}

public class StrangeCat : BaseCat
{
    public StrangeCat(bool canMiau) : base(canMiau) { }
}

public class MuteCat : BaseCat
{
    public MuteCat() : base(false) { }
}

public class LoudCat : BaseCat
{
    public LoudCat() : base(true) { }
}


Then when you kick the cat, it will respond appropriately with minimal duplication of logic. I would avoid an abstract class unless the inheritors are going to implement significant logic differences, your example doesn't show that (though your real situation may have that).

public void KickCat(BaseCat)
{
    Foot.StraightTowardsCat(BaseCat, Anatomy.Rear);
    BaseCat.SayMiau();
}

...

KickCat(new MuteCat()); // I love this cat, it's like it doesn't even care!
KickCat(new LoudCat()); // Whines, blah.
KickCat(new StrangeCat(true)); // Whines this time.
KickCat(new StrangeCat(false)); // Doesn't whine this time! Woot!

Code Snippets

public class BaseCat
{
    protected bool _canMiau;

    // if you wanted abstract so BaseCat could not be constructed make the constructor protected
    protected BaseCat(bool canMiau) { _canMiau = canMiau; }

    public virtual void SayMiau()
    {
        if (_canMiau)
        {
            Console.WriteLine("MIAU!");
        }
    }
}

public class StrangeCat : BaseCat
{
    public StrangeCat(bool canMiau) : base(canMiau) { }
}

public class MuteCat : BaseCat
{
    public MuteCat() : base(false) { }
}

public class LoudCat : BaseCat
{
    public LoudCat() : base(true) { }
}
public void KickCat(BaseCat)
{
    Foot.StraightTowardsCat(BaseCat, Anatomy.Rear);
    BaseCat.SayMiau();
}

...

KickCat(new MuteCat()); // I love this cat, it's like it doesn't even care!
KickCat(new LoudCat()); // Whines, blah.
KickCat(new StrangeCat(true)); // Whines this time.
KickCat(new StrangeCat(false)); // Doesn't whine this time! Woot!

Context

StackExchange Code Review Q#3705, answer score: 2

Revisions (0)

No revisions yet.