Abstract classes and interfaces in a game inventory system

After reading a lot about interfaces, which I'm new to, I've tried implementing it in my inventory system for a game I'm working on. I have managed to create something proof of concept which seems to work as intended, but I wanted to check with more experienced C# programmers if this is the correct/best approach?

The player has an inventory, an inventory has several slots, and each slots can have several items. The player can use any of the different items (or, more precisely, item types), but how they are used differs; food items can be eaten, weapon items can be equipped etc.

So I have started with this:

public interface IUser {
void Use ( Item usableItem );
}

public class Player : IUser {
public float Energy { get; set; }

public Player () {
Energy = 0.0f;
}

public void Use ( Item usableItem ) {
usableItem.Use ( this );
}
}

public interface IUsable {
void Use ( Player player );
}

public abstract class Item : IUsable {
public string Name { get; set; }

public Item () {
Name = "ITEM WITH NO NAME";
}

public virtual void Use ( Player player ) {
// Nothing here?
}
}

public abstract class Food : Item {
public float Energy { get; set; }

public Food () {
Name = "FOOD WITH NO NAME";
Energy = 1.0f;
}

public override void Use ( Player player ) {
player.Energy += Energy;
}
}

public class Meat : Food {
public Meat () {
Name = "Meat";
Energy = 10.0f;
}
}


Does this at all make sense?

I think your design introduces a few more dependencies than it should.

Let's have a look at your IUsable interface

public interface IUsable {
    void Use ( Player player );
}

Do you really need to have a dependency on Player? Why don't you just require it to depend on IUser?

I'd also completely get rid of the Item class. What do you need it for? If you want an abstract class to represent a NamedItem you can create it but you should do it in the following way:

public abstract class NamedItem implements IUsable
{
    public string Name {get; private set;} // Better to keep it immutable

    protected NamedItem(string name)
    {
        Name = name;
    }

    public abstract void Use(IUser user);
    /*
     * This way we don't need to provide a 
     * default implementation and your subclasses
     * will be forced to provide it.
     */
}

I think you should probably also revise who should be in charge of implementing the Use method. From the names you're using it is quite clear that it is more natural to think at a player that uses an item. That's fine but what your code suggests is that different objects should be used in different ways, hence you need to implement it on the specific Item instance.

Does spreading the logic on how a player uses an item across a lot of different items makes sense in your project? If you don't want to do it you might want to have a look at dobule dispatch as a possible way to retain all the behaviours on player instead than on objects. If you do double dispatch you can have interfaces such as IFood and then have a method in Player with this signature void User(IFood food). IFood should expose a way to determine how many energy is added by the food to the player and the mutation of the state of the player should happen in the specific method you just created.

If you show us how you intend to use those classes we can help you find the best way to design them.

StackExchange Code Review Q#68654, answer score: 4