0

I'm making a naval warfare strategy game, based on a boardgame, in C# and having difficulty finding a good way to construct my complex ship objects (I am a beginner in the language and OOP).

Currently, each ship class inherits from an abstract base Ship class that implements the interface IShip (code below). This interface defines the properties and behaviours common to all ships. I have attempted to reduce complexity by grouping related properties into their own objects (e.g., one models the crew, another models the rigging, etc.). As these properties are required for a given ship to function properly, I would like to initialise them in the ship constructor. I have reduced the number of constructor parameters to just a single ShipProperties object, which gets passed into a simple ship factory method to simplify ship construction and reduce code repetition. Ultimately, this property object will be set, using object initialisers for clarity, by reading game scenarios from a JSON file.

I feel there must be a better solution to this issue, however, so would appreciate feedback on my design.

Ship interface:

public interface IShip
{
    Identifiers Identifiers { get; }
    Stats Stats { get; }
    Crew Crew { get; }
    Rigging Rigging { get; }
    Armaments Armaments { get; }
    MovementAbilities MovementAbilities { get; }

    void MoveTo(Coordinates coordinates);
    void GrappleWith(IShip ship);
    void UngrappleFrom(IShip ship);
    void Board(IShip ship);
    void LoadBroadsidesWith(ShotType shotType);
    void FireBroadside();
    void Unfoul();
}

Abstract base ship class, with default implementations of ship interface:

public abstract class Ship : IShip
    {
        public Identifiers Identifiers { get; }
        public Stats Stats { get; }
        public Crew Crew { get; }
        public Rigging Rigging { get; }
        public Armaments Armaments { get; }
        public MovementAbilities MovementAbilities { get; }

        public Ship(ShipProperties shipProperties)
        {
            Identifiers = shipProperties.Identifiers;
            Stats = shipProperties.Stats;
            Crew = shipProperties.Crew;
            Rigging = shipProperties.Rigging;
            Armaments = shipProperties.Armaments;
            MovementAbilities = shipProperties.MovementAbilities;
        }

        public void MoveTo(Coordinates coordinates) { }
        public void GrappleWith(IShip ship) { }
        public void UngrappleFrom(IShip ship) { }
        public void Board(IShip ship) { }
        public void LoadBroadsidesWith(ShotType shotType) { }
        public void FireBroadside() { }
        public void Unfoul() { }
    }

Ship properties record:

public record ShipProperties
{
    public Identifiers Identifiers { get; init; }
    public Stats Stats { get; init; }
    public Crew Crew { get; init; }
    public Rigging Rigging { get; init; }
    public Armaments Armaments { get; init; }
    public MovementAbilities MovementAbilities { get; init; }
}

Ship property objects:

public class Identifiers
{
    public string Name { get; }
    public ShipClass Class { get; } // is an enum
    public Nationality Nationality { get; } // is an enum
    public int ID { get; }

    public Identifiers(string name, ShipClass @class, Nationality nationality) // not every ship has an ID in every scenario
    {
        Name = name;
        Class = @class;
        Nationality = nationality;
    }

    public Identifiers(string name, ShipClass @class, Nationality nationality, int id)
        : this(name, @class, nationality)
    {
        ID = id;
    }
}

public class Stats
{
    public int PointValue { get; }
    public int HullHP { get; private set; }
    public int HullDepth { get; }

    public Stats(int pointValue, int hullHP, int hullDepth)
    {
        PointValue = pointValue;
        HullHP = hullHP;
        Depth = hullDepth;
    }
}

public class Crew
{
    public CrewQuality Quality { get; } // is an enum
    public int[] SectionStrengths { get; set; }

    public Crew(CrewQuality quality, params int[] sectionStrengths)
    {
        Quality = quality;

        SectionStrengths = new int[sectionStrengths.Length];
        for (int i = 0; i < sectionStrengths.Length; i++)
        {
            SectionStrengths[i] = sectionStrengths[i];
        }
    }
}

public class Rigging
{
    public int[] RiggingHPPerMast { get; set; }
    public Rigging(params int[] riggingHPPerMast)
    {
        RiggingHPPerMast = new int[riggingHPPerMast.Length];
        for (int i = 0; i < riggingHPPerMast.Length; i++)
        {
            RiggingHPPerMast[i] = riggingHPPerMast[i];
        }
    }
}

public class Armaments
{
    public ShotType ShotType { get; private set; } // is an enum
    public Guns Guns {get; private set;}
    public Carronades Carronades { get; private set; }

    public Armaments(Guns guns)
    {
        Guns = guns;
    }

    public Armaments(Guns guns, Carronades carronades) : this(guns)
    {
        Carronades = carronades;
    }
}

public class MovementAbilities
{
    public int TurningAbility { get; private set; }
    public int BattleSailSpeed { get; private set; }
    public int FullSailSpeed { get; private set; }

    public MovementAbilities(int turningAbility, int battleSailSpeed) // for basic game rules
    {
        TurningAbility = turningAbility;
        BattleSailSpeed = battleSailSpeed;
    }

    public MovementAbilities(int turningAbility, int battleSailSpeed, int fullSailSpeed)
        : this(turningAbility, battleSailSpeed)
    {
        FullSailSpeed = fullSailSpeed;
    }
}

Ship factory:

public class ShipFactory
{
    public IShip CreateShip(ShipProperties shipProperties)
    {
        return shipProperties.Identifiers.Class switch
        {
            ShipClass.ShipOfTheLine      => new ShipOfTheLine(shipProperties),
            ShipClass.LargeShipOfTheLine => new LargeShipOfTheLine(shipProperties),
            ShipClass.Frigate            => new Frigate(shipProperties),
            ShipClass.LargeFrigate       => new LargeFrigate(shipProperties),
            ShipClass.Corvette           => new Corvette(shipProperties),
            ShipClass.Brig               => new Brig(shipProperties),
            ShipClass.Sloop              => new Sloop(shipProperties),
            ShipClass.Schooner           => new Schooner(shipProperties),
            ShipClass.Gondola            => new Gondola(shipProperties),
            ShipClass.Galley             => new Galley(shipProperties),
            ShipClass.Radeau             => new Radeau(shipProperties),
            ShipClass.Gunboat            => new Gunboat(shipProperties),
            ShipClass.Privateer          => new Privateer(shipProperties),
            ShipClass.Merchantman        => new Merchantman(shipProperties),
            ShipClass.FireShip           => new FireShip(shipProperties),
            ShipClass.BombKetch          => new BombKetch(shipProperties),
            _                            => throw new NotImplementedException()
        };
    }
}
7
  • Doesn't C# have reflection? I usually feel that classes like your ShipFactory are much better written with a judicious invoke() than with a tedious denormalized listing like this. Jun 20 at 11:31
  • @KilianFoth I'm only vaguely aware of reflection, which C# does support, but I've never used it. I'll look into it, thanks. Jun 20 at 11:38
  • What is the purpose of the more specific types of ship? If they are overriding the methods from the abstract class, you'll need to make them virtual or (more likely) abstract if you need them to provide their own behaviour. Jun 20 at 19:56
  • @VisualMelon I am still in the early design stages so am not currently focusing on implementation. The purpose of the abstract base class is to provide default implementations for ship behaviour defined by the ship interface. I will probably want the option to override them in some cases, so will most likely make them virtual. I didn't think that would be relevant to my question, however. Jun 20 at 21:57
  • 1
    Required reading: ericlippert.com/2015/04/27/wizards-and-warriors-part-one
    – nvoigt
    Jun 22 at 14:22
0

If you want to stick to OOP, there are multiple issues with your current approach. Although, actually shipping a game is a much higher priority that in other industries, so whatever floats your boat. All the nice programming patterns usually have trade-offs, it's up to you to decide if they are worth it.


The main problem here is that the config is mixed with the state.

You may notice that some properties define ship's abilities and capabilities and limits, these properties will be read from the data files and will not change ever. On other hand, the properties that define a ship's current state will change during ship's lifetime, and initially they are set from the ship's config (the previously mentioned kind of properties).

Normally, things that change together and are used together must go together. Which suggests that ship's state must belong to the ship itself, while the config must be extracted into it's own class.

An easy way to build a data-only object type that's also trivially de-serializable, is to rely on init-only setters. All you need to do is to list all the properties as {get; init;}, no constructors needed.

In future, you probably want to componentize your ships, but for now there is not much point in separation of config into smaller classes. You'd think that decreases the complexity, but having to manage a class hierarchy is actually more complex that just having a single class.

Considering all of the above, here's how the relevant code could look like:

public record ShipConfig
{
    // ==== identifiers
    public string Name { get; init; }
    public ShipClass Class { get; init; }
    public Nationality Nationality { get; init; }
    public int ID { get; init; }

    // ==== stats
    public int PointValue { get; init; }
    public int MaxHullHP { get; init; } // NOTE: renamed
    public int HullDepth { get; init; }

    // ==== crew
    public CrewQuality Quality { get; init; }
    public int[] SectionStrengths { get; init; }

    // ==== rigging
    public int[] RiggingHPPerMast { get; init; }

    // ==== armaments
    public ShotType ShotType { get; init; }
    public Guns Guns { get; init; }
    public Carronades Carronades { get; init; }

    // ==== movement
    public int TurningAbility { get; init; }
    public int BattleSailSpeed { get; init; }
    public int FullSailSpeed { get; init; }
}

public abstract class Ship : IShip
{
    // state is *protected* to allow derived classes to access it
    protected int hullHP;
    protected int[] sectionStrengths;
    protected int[] riggingHPPerMast;
    protected Guns guns;
    protected Carronades carronades;
    protected int turningAbility;
    protected int battleSailSpeed;
    protected int fullSailSpeed;

    public Ship(ShipConfig config)
    {
        Config = config;
        hullHP = config.MaxHullHP;
        sectionStrengths = config.SectionStrengths;
        riggingHPPerMast = config.RiggingHPPerMast;
        guns = config.Guns;
        carronades = config.Carronades;
        turningAbility = config.TurningAbility;
        battleSailSpeed = config.BattleSailSpeed;
        fullSailSpeed = config.FullSailSpeed;
    }

    // interface changes are implied

    public ShipConfig Config { get; }
    public int HullHP => hullHP;
    public IReadOnlyList<int> SectionStrengths => sectionStrengths; // NOTE: read-only collection type
    public IReadOnlyList<int> RiggingHPPerMast => riggingHPPerMast; // NOTE: read-only collection type
    public Guns Guns => guns;
    public Carronades Carronades => carronades;
    public int TurningAbility => turningAbility;
    public int BattleSailSpeed => battleSailSpeed;
    public int FullSailSpeed => fullSailSpeed;

    // methods omitted
}
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.