patterncsharpMinor

Armor and Weapon classes for a game

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

weapongameforclassesandarmor

Problem

I have a 2 classes Armor and Weapon in my game, that inherit from the abstract class Gear:

public class Gear {
    public int price;
    // Other common attributes between armors and weapons
}

public class Weapon : Gear {
    public int damage = 10;
    public int weight = 10;
    public int range = 10;
}

public class Armor : Gear {
    // Defense against different kinds of attack
    public int blunt, slash, ranged;
}

Then I have a class GearButton which is nothing more than a button, with a piece of gear associated. I inherit from this class to create Buttons to Buy, Equip or Upgrade this items, for instance.

This is 2 examples of it:

BuyArmorButton.cs

public class BuyArmorButton : GearButton {

    // ...

    void BuyItem() {
        bool bought = GameSession.Buy((Armor) this.gear);
        if (bought) this.ShowMessage('Buy successful');
    }

    void Update() {
        bool can_buy = GameSession.CanBuy((Armor) this.gear);
        this.EnableButton(can_buy);
    }

    // ...
}

BuyWeaponButton.cs

public class BuyWeaponButton : GearButton {

    // ...

    void BuyItem() {
        bool bought = GameSession.Buy((Weapon) this.gear);
        if (bought) this.ShowMessage('Buy successful');
    }

    void Update() {
        bool can_buy = GameSession.CanBuy((Weapon) this.gear);
        this.EnableButton(can_buy);
    }

    // ...
}

I have added the methods from GameSession, which probably might also need to be tweaked, for this to work.

```
public static bool Buy(Armor armor) {
if (!CanBuy(armor)) return false;

data.money -= armor.price;
data.armorsPurchased.Add(armor);
return true;
}

public static bool Buy(Weapon weapon) {
if (!CanBuy(weapon)) return false;

data.money -= weapon.price;
data.weaponsPurchased.Add(weapon);
return true;
}

public static bool CanBuy(Armor armor) {
if (data.armorsPurchased.Contains(armor))
return false;

if (armor.price > data.money)

Solution

Once various responsibilites (functionality) is separated into appropriate classes, inheritance and using subclasses is not a problem.

Rethink your core classes. Don't make client code do the work - that's a guarantee of duplication. A good class hides state and exposes functionality. Even basic functionality like overriding Equals pays off. "Do not make client code do the work for that class". Repeat this a million times. Oh, I expect you will also avoid tons of debugging.

Some methods are overloaded. The point is when you see a need for something different put that in the proper class. Don't be lazy and write it in the calling class because it's easy and quick.

SPOILER: gear and gear collections are just data structures. Abstract things. A Warrior has his personal stuff (a gear collection) and an Armory (a really big gear collection) is where he buys his stuff.

Gear enum

Looks like you care only if some gear is a weapon, armor, etc. not a specific object.

public enum GearType { undefined, Weapon, Armor }

Make Gear abstract

And give it all the common gear behavior.

public abstract class Gear {
    protected decimal price { get; set; }
    public GearType Type { get; protected set; } // set only in a constructor

    public virtual bool IsSameKind( Gear otherGear ) {
        return this.Equals( otherGear );
    }

    // I thought of this as I was working on the collection class below
    public virtual bool IsSameKind( GearType thisKind ) { return this.Type == thisKind; }

    // any armor is armor. any weapon is weapon....
    /// 
    /// Equates on Type property. 
    ///
    public override Equals( Object otherGear ) {
       if( otherGear == null ) return false;

       otherGear = otherGear as Gear;
       if( otherGear == null ) return false;

       return this.Type == otherGear.Type;
    }
}

Use Constructors

To guarantee valid, complete, correct objects.

public class Armor : Gear {
   protected int blunt { get; set; }
   protected int slash { get; set; }
   protected int range { get; set; }

   public Armor ( decimal price, int blunt, int slash, int range ) {
      this.Type = GearType.Armor;
      // set other properties here
   }
}

Custom Collections

Solves your upcast issue.

Also higher level code will make more sense, is cleaner, clearer, and simpler.

publc class GearCollection {
   protected List Armory { get; set; }

   public GearCollection() { Armory = new List(); }

   public void Add ( Gear thing ) {
       if ( thing == null ) return;
       Armory.Add( thing );
   }

   // Passing in a Gear reference. The OBJECT itself will be
   // a "Weapon", "Armor", etc.
   public bool Contains( Gear otherThing ) {
       if ( otherThing == null ) return false;
       if ( Armory.Contains( otherThing ) ) return true;  // uses equals override.
       return false;
   }

   public bool Contains( GearType thisGear ){
      bool gotIt = false;

      foreach( var item in Armory ) {
         if( item.IsSameKind( thisGear ) {
            gotIt = true;
            break;
         }
      }            
      return gotIt;
  }

   public GearCollection GetAll( GearType thisKind ){
      GearCollection sameGear = new GearCollection();

      foreach( var item in Armory ) {
         if( item.IsKind ( thisKind ) {
            sameGear.Add( item );
         }
      }
      return sameGear;
   }

   // this method makes THIS collection object mutable.
   // a discussion for another time.
   public GearCollection GetAll( Gear likeThis ){
      GearCollection sameGear = new GearCollection();

      foreach( var item in Armory ) {
         if( item.Equals( likeThis ) ) sameGear.Add( item );
      }
      return sameGear;
   }
}

Separation of concerns

Why would a button have its own gear? That's a code smell to me. I'd expect event handlers to call methods on domain objects like Warrior or AmmoDump or WeaponCache. Those are things I expect to have Gear, not the UI controls.

```
public class Warrior {
protected GearCollection MyGear { get; set; }
public Warrior() { MyGear = new GearCollection(); }
public Warrior( GearCollection standardIssue ) {
MyGear = standardIssue?? new GearCollection();
}
}

// The Armory is where you buy your gear. A gear collection is just
// a data structure => separation of concerns.
public class Armory {
protected GearCollection Inventory { get; set; }

// optional parameter so I don't have to write
// a contructor overload.
public Armory( GearCollection newInventory = null ){
Inventory = newInventory ?? new GearCollection();
}

// renamed "CanBuy".
public bool IsInStock( GearType gearKind ) {
return Inventory.Contains( gearKind );
}

public bool IsInStock( Gear likeThis ) {
return Inventory.Contains( likeThis ) {
}

public Gear Buy( GearType something, decimal cashOnly ) {
if( !IsInStock( something ) ) return null;

Gear newBling = Inventor

Code Snippets

public enum GearType { undefined, Weapon, Armor }

public abstract class Gear {
    protected decimal price { get; set; }
    public GearType Type { get; protected set; } // set only in a constructor

    public virtual bool IsSameKind( Gear otherGear ) {
        return this.Equals( otherGear );
    }

    // I thought of this as I was working on the collection class below
    public virtual bool IsSameKind( GearType thisKind ) { return this.Type == thisKind; }

    // any armor is armor. any weapon is weapon....
    /// <summary>
    /// Equates on Type property. 
    ///</summary>
    public override Equals( Object otherGear ) {
       if( otherGear == null ) return false;

       otherGear = otherGear as Gear;
       if( otherGear == null ) return false;

       return this.Type == otherGear.Type;
    }
}

public class Armor : Gear {
   protected int blunt { get; set; }
   protected int slash { get; set; }
   protected int range { get; set; }

   public Armor ( decimal price, int blunt, int slash, int range ) {
      this.Type = GearType.Armor;
      // set other properties here
   }
}

publc class GearCollection {
   protected List<Gear> Armory { get; set; }

   public GearCollection() { Armory = new List<Gear>(); }

   public void Add ( Gear thing ) {
       if ( thing == null ) return;
       Armory.Add( thing );
   }

   // Passing in a Gear reference. The OBJECT itself will be
   // a "Weapon", "Armor", etc.
   public bool Contains( Gear otherThing ) {
       if ( otherThing == null ) return false;
       if ( Armory.Contains( otherThing ) ) return true;  // uses equals override.
       return false;
   }

   public bool Contains( GearType thisGear ){
      bool gotIt = false;

      foreach( var item in Armory ) {
         if( item.IsSameKind( thisGear ) {
            gotIt = true;
            break;
         }
      }            
      return gotIt;
  }

   public GearCollection GetAll( GearType thisKind ){
      GearCollection sameGear = new GearCollection();

      foreach( var item in Armory ) {
         if( item.IsKind ( thisKind ) {
            sameGear.Add( item );
         }
      }
      return sameGear;
   }

   // this method makes THIS collection object mutable.
   // a discussion for another time.
   public GearCollection GetAll( Gear likeThis ){
      GearCollection sameGear = new GearCollection();

      foreach( var item in Armory ) {
         if( item.Equals( likeThis ) ) sameGear.Add( item );
      }
      return sameGear;
   }
}

public class Warrior {
    protected GearCollection MyGear { get; set; }
    public Warrior() { MyGear = new GearCollection(); }
    public Warrior( GearCollection standardIssue ) {
       MyGear = standardIssue?? new GearCollection();
    }
}

// The Armory is where you buy your gear. A gear collection is just
// a data structure => separation of concerns.
public class Armory {
   protected GearCollection Inventory { get; set; }

   // optional parameter so I don't have to write 
   // a contructor overload.
   public Armory( GearCollection newInventory = null ){
      Inventory = newInventory ?? new GearCollection();
   }

  // renamed "CanBuy". 
   public bool IsInStock( GearType gearKind ) {
       return Inventory.Contains( gearKind );
   }

   public bool IsInStock( Gear likeThis ) {
      return Inventory.Contains( likeThis ) {
   }

   public Gear Buy( GearType something, decimal cashOnly ) {
      if( !IsInStock( something ) )  return null;

      Gear newBling = Inventory.Remove( something );  // didn't write that method yet.
      if( cashOnly < newBling.Price ) return null;

      return newBling;
   }

   public void AddInventory ( GearCollection newShipment ) {
      if( newShipment == null ) return;
      Inventory.AddRange( newShipment );
   }   
}

Context

StackExchange Code Review Q#156914, answer score: 9

Revisions (0)

No revisions yet.