Is it possible to override a method with a derived parameter instead of a base one?

Question

I'm stuck in this situation where:

1. I have an abstract class called Ammo, with AmmoBox and Clip as children.
2. I have an abstract class called Weapon, with Firearm and Melee as children.
3. Firearm is abstract, with ClipWeapon and ShellWeapon as children.
4. Inside Firearm, there's a void Reload(Ammo ammo);

The problem is that, a ClipWeapon could use both a Clip and an AmmoBox to reload:

public override void Reload(Ammo ammo)
{
    if (ammo is Clip)
    {
        SwapClips(ammo as Clip);
    }
    else if (ammo is AmmoBox)
    {
        var ammoBox = ammo as AmmoBox;
        // AddBullets returns how many bullets has left from its parameter
        ammoBox.Set(clip.AddBullets(ammoBox.nBullets));
    }
}

But a ShellWeapon, could only use an AmmoBox to reload. I could do this:

public override void Reload(Ammo ammo)
{
    if (ammo is AmmoBox)
    {
        // reload...
    }
}

But this is bad because, even though I'm checking to make sure it's of type AmmoBox, from the outside, it appears like a ShellWeapon could take a Clip as well, since a Clip is Ammo as well.

Or, I could remove Reload from Firearm, and put it both ClipWeapon and ShellWeapon with the specific params I need, but doing so I will lose the benefits of Polymorphism, which is not what I want to.

Wouldn't it be optimal, if I could override Reload inside ShellWeapon like this:

public override void Reload(AmmoBox ammoBox)
{
   // reload ... 
}

Of course I tried it, and it didn't work, I got an error saying the signature must match or something, but shouldn't this be valid 'logically'? since AmmoBox is a Ammo?

How should I get around this? And in general, is my design correct? (Note I was using interfaces IClipWeapon and IShellWeapon but I ran into trouble, so I moved to using classes instead)

Thanks in advance.

Solution 1

but shouldn't this be valid 'logically'?

No. Your interface says that the caller can pass in any Ammo - where you're restricting it to require an AmmoBox, which is more specific.

What would you expect to happen if someone were to write:

Firearm firearm = new ShellWeapon();
firearm.Reload(new Ammo());

? That should be entirely valid code - so do you want it to blow up at execution time? Half the point of static typing is to avoid that sort of problem.

You could make Firearm generic in the type of ammo is uses:

public abstract class Firearm<TAmmo> : Weapon where TAmmo : Ammo
{
    public abstract void Reload(TAmmo ammo);
}

Then:

public class ShellWeapon : Firearm<AmmoBox>

That may or may not be a useful way of doing things, but it's at least worth considering.

Solution 2

You can use composition with interface extensions instead of multiple-inheritance:

class Ammo {}
class Clip : Ammo {}
class AmmoBox : Ammo {}

class Firearm {}
interface IClipReloadable {}
interface IAmmoBoxReloadable {}

class ClipWeapon : Firearm, IClipReloadable, IAmmoBoxReloadable {}
class AmmoBoxWeapon : Firearm, IAmmoBoxReloadable {}

static class IClipReloadExtension {
    public static void Reload(this IClipReloadable firearm, Clip ammo) {}
}

static class IAmmoBoxReloadExtension {
    public static void Reload(this IAmmoBoxReloadable firearm, AmmoBox ammo) {}
}

So that you will have 2 definitions of Reload() method with Clip and AmmoBox as arguments in ClipWeapon and only 1 Reload() method in AmmoBoxWeapon class with AmmoBox argument.

var ammoBox = new AmmoBox();
var clip = new Clip();

var clipWeapon = new ClipWeapon();
clipWeapon.Reload(ammoBox);
clipWeapon.Reload(clip);

var ammoBoxWeapon = new AmmoBoxWeapon();
ammoBoxWeapon.Reload(ammoBox);

And if you try pass Clip to AmmoBoxWeapon.Reload you will get an error:

ammoBoxWeapon.Reload(clip); // <- ERROR at compile time

Solution 3

The problem with which you are wrestling comes from the need to call a different implementation based on the run-time types of both the ammo and the weapon. Essentially, the action of reloading needs to be "virtual" with respect to two, not one, object. This problem is called double dispatch.

One way to address it would be creating a visitor-like construct:

abstract class Ammo {
    public virtual void AddToShellWeapon(ShellWeapon weapon) {
        throw new ApplicationException("Ammo cannot be added to shell weapon.");
    }
    public virtual void AddToClipWeapon(ClipWeapon weapon) {
        throw new ApplicationException("Ammo cannot be added to clip weapon.");
    }
}
class AmmoBox : Ammo {
    public override void AddToShellWeapon(ShellWeapon weapon) {
        ...
    }
    public override void AddToClipWeapon(ClipWeapon weapon) {
        ...
    }
}
class Clip : Ammo {
    public override void AddToClipWeapon(ClipWeapon weapon) {
        ...
    }
}
abstract class Weapon {
    public abstract void Reload(Ammo ammo);
}
class ShellWeapon : Weapon {
    public void Reload(Ammo ammo) {
        ammo.AddToShellWeapon(this);
    }
}
class ClipWeapon : Weapon {
    public void Reload(Ammo ammo) {
        ammo.AddToClipWeapon(this);
    }
}

"The magic" happens in the implementations of Reload of the weapon subclasses: rather than deciding what kind of ammo they get, they let the ammo itself do "the second leg" of double dispatch, and call whatever method is appropriate, because their AddTo...Weapon methods know both their own type, and the type of the weapon into which they are being reloaded.

Solution 4

I think, that it's perfectly fine to check, whether passed Ammo is of valid type. The similar situation is, when function accepts a Stream, but internally checks, whether it is seekable or writeable - depending on its requirements.

View more solutions

Comments

[vexe]

The problem is, since both AddToShellWeapon and AddToClipWeapon are both abstract, they must get defined in whoever's inheriting Ammo, so inside of Clip there's AddToShellWeapon, which is redundant. Correct me if I'm wrong.

[Sergey Kalinichenko]

@VeXe You don't need to make them abstract - you can make them virtual, and have them throw an exception by default, saying "this kind of amo cannot be added to that kind of weapon". I edited the answer to illustrate the approach.

[vexe]

How can re-write the generic definition you wrote but for Firearm instead of Weapon? (because Shell and Clip weapons inherit from Firearm and not Weapon directly) I tried something like this but obviously that's not it is: public abstract class Firearm <AmmoType> where AmmoType :Ammo : Weapon XD

[vexe]

It's a nice idea, going the other way, but if I were to do that, then why wouldn't I just use my first solution and add an else if (ammo is Clip) -> throw exception? What does your method provide more, so that I would consider it instead of mine?

[Jon Skeet]

@VeXe: You need to put the : Weapon before the where AmmoType : Ammo. I'll update my answer to show this. I would strongly recommend that you use the T prefix for type parameters though.

[Sergey Kalinichenko]

@VeXe "Why not just continue the chain of if-then-elses" is a very valid question. One advantage of this approach is that it makes the if-then-else chain visible in the interface of the class: implementors of new subclasses of Amo will see what methods they must override, as opposed to having to ask you or know from prior experience that they need to go to a chain of ifs and add their new subclass to that chain in every single subclass of the weapon with which they suppose to interact.

[vexe]

This is EXACTLY what I'm trying to do, but the way you're doing it is a little bit beyond my knowledge. I didn't really get what's up with the extensions, and what the 'this' is doing... perhaps I go check some tutorials on this interface extension and take a look at your solution again. What's supposed to fit inside the interfaces? Sorry but it's a bit blurry to me at the moment.

[vexe]

I think this is best, but may I ask why did you say it may not be a useful way of doing things?

[Jon Skeet]

@VeXe: Introducing generics can be a pain in other ways - but we don't know enough about your situation to know whether it's the best solution for your specific case. It may well be.

[vexe]

OK, honestly your solution, @dasblinkenlight's and simply Spook's (which opted for my first approach) are all good, and I can't simply decide which one to go for and accept, nor use in my situation. I guess I'm gonna have to go back, mess with the 3 solution and decide then. Great help, thanks a lot guys :)

[Mikhail Churbanov]

@VeXe If you need only Reload method you can leave interfaces empty, the extension will do all you need. Take a look on them it's quite useful feature of C# for functional extension of classes.

[vexe]

I've accepted this one, because it made the most sense for me, and fitted right in, I didn't have to edit/redesign any logic. Sorry @dasblinkenlight, your method was probably correct programming/design-wise and would work fine, but it didn't make sense for me to make the Ammo itself be responsible for the adding and messing with the Firearm, I think it should be the Firearm who's responsible for reloading and managing itself, from within itself :)

[dragonfly02]

@JonSkeet Was it ever a compile error in .NET if using a different argument name in a derived class method when overriding a base method?

[Jon Skeet]

@stt106: Do you mean a different parameter name? No, I don't think that's ever been an error.