Want to avoid returning writable references inside of a complex read-only class (game state)

Question

I'm writing a little game, and the game has a State class to keep track of game state. The State class is only intended to be modifiable by Commands (command pattern). The State class includes lists of other classes - e.g. a Faction class, which contains members like resources, a list of owned Units, etc.

How can I make the deep innards of State readable from other classes, without also leaking writable references inside of State itself?

Currently, I have specialized getters like State.GetOwnerOfUnitAtLocation(x), which only return safe values (int factionid etc.), but I am beginning to need a lot of these, and the class is getting really unwieldy. I would prefer to have those methods in more appropriate locations (Map.Units.GetOwner(x) or something), but I don't know how to expose the internals of State to other classes in a safe way.

Relatedly, Command is an interface that currently lives inside of the State class, along with all the actual commands that implement it, so that it can modify private members of State. Is there a better way to implement this?

Edit: A selection of code from State to illustrate the first issue:

public partial class State
{
    public int Turn {get; private set;} = 1;
    
    private Dictionary<Vector2, FactionsMgr.Faction> _unit_map = new Dictionary<Vector2, FactionsMgr.Faction>();

    public int GetUnitRemainingMobility(Vector2 pos)
    {
        if (IsUnitAt(pos))
        {
            FactionsMgr.Faction owner = _unit_map[pos];
            int taken_movement = owner.units[pos]._taken_movement;
            int max_mobility = UnitsMgr.GetMaxMobility(owner.units[pos].type);
            return max_mobility - taken_movement;
        }
        else
        {
            GD.Print("Warning: GetUnitRemainingMobility asked for unknown unit: ", pos);
            return -1;
        }
    }
}

Can you explain, with the help of some code, exactly how exposing `Map.Units.GetOwner(x)` is "not safe"? — Sweeper, Aug 22 '21 at 08:04
@OlivierRogier `State` consists of private members. For example: `private List _factions; private Dictionary _unit_map;` (simplifying slightly to fit into a comment). — Quintus, Aug 22 '21 at 08:07
@Sweeper, I'm unsure I understand your question. What I'd like to do is to expose the internals of `State` in a read-only (this is what I mean by safe) way to other classes, such as `Map`. Currently, `Map` cannot see inside of `State` without calling on `State`'s very many custom getter methods, which return by value. — Quintus, Aug 22 '21 at 08:11
Yep you understood my question correctly. One solution is to just write an read-only interface for each of the mutable things in `State`, like how `IReadOnlyList` is the read-only version of `IList`. Expose the read-only interfaces, and keep the implementations private, only accessible by the inner class `Command`. If you show some code of `State`, I can show you how you'd do this exactly. — Sweeper, Aug 22 '21 at 08:18
@Sweeper I've pulled out some code from `State` for you - is this helpful to get an idea of the current implementation? — Quintus, Aug 22 '21 at 08:31
Does this answer your question? [Is there an easy way to make an immutable version of a class?](https://stackoverflow.com/questions/26105608/) and [How do I create an immutable Class?](https://stackoverflow.com/questions/352471/) and [How to make a class with List<> member immutable?](https://stackoverflow.com/questions/37169470/) — , Aug 22 '21 at 10:28

Sweeper · Accepted Answer · 2021-08-22T09:24:39.410

Since FactionsMgr.Faction is mutable, let's suppose that it has writable properties like this:

class Faction {
    public int Foo { get; set; }
    public string Bar { get; set; }
    public float Baz { get; set; }
    public SomeOtherMutableThing AnotherThing { get; set; }
}

You should create a corresponding read only interface for it, and make Faction implement it:

interface IReadOnlyFaction {
    // exclude all members that can change Faction's state
    int Foo { get; }
    string Bar { get; }
    float Baz { get; }
    IReadOnlySomeOtherMutableThing AnotherThing { get; }
}

interface IReadOnlySomeOtherMutableThing {
    // do the same thing there...
}

class Faction: IReadOnlyFaction {
    public int Foo { get; set; }
    public string Bar { get; set; }
    public float Baz { get; set; }
    public SomeOtherMutableThing AnotherThing { get; set; }

    // you need an explicit interface implementation here, unless you are using C# 9
    IReadOnlySomeOtherMutableThing IReadOnlyFaction.AnotherThing => AnotherThing;
}

Then, you can declare public members in State as of type IReadOnlyFaction, and delegate them to a private member of type Faction. The private member is also what the Command class will modify.

private Faction someFaction;
public IReadOnlyFaction SomeFaction => someFaction;

That is the general case. However, if you have collections of these mutable types, like your dictionary of _unit_map, you would need to do a bit more work.

You would still have a public read only member and a private mutable member, but the delegating process is less straightforward. You would need a wrapper.

private Dictionary<Vector2, FactionsMgr.Faction> _unit_map = new();

public IReadOnlyDictionary<Vector2, IReadOnlyFaction> UnitMap;

// ...

// in the constructor of State...

// using the ReadOnlyDictionaryWrapper class from the linked answer
UnitMap = new ReadOnlyDictionaryWrapper<Vector2, FactionsMgr.Faction, IReadOnlyFaction>(_unit_map);

If the `Faction` class contains a Dictionary of builtin types (e.g. int to int), how do I include that in the `IReadOnlyFaction` interface? If I use an `IReadOnlyDictionary` in the corresponding part of `IReadOnlyFaction`, then `Faction` gives an error because it is not implementing the interface's expected return type. Is this a matter of writing a custom `{get;}` for one or both? — Quintus, Aug 23 '21 at 02:26
@Quintus You would do a transformation similar to `SomeOtherMutableThing`, except that there is already a `IReadOnlyDictionary` built in, so you don't need to create a new interface. Use an explicit interface implementation in `Faction` to implement the `IReadOnlyDictionary` property, as they will have the same name. — Sweeper, Aug 23 '21 at 02:29
So you'll need to add something like `IReadOnlyDictionary IReadOnlyFaction.SomeDictionary => SomeDictionary;` in `Faction` to implement `IReadOnlyFaction`. @Quintus — Sweeper, Aug 23 '21 at 02:30

Want to avoid returning writable references inside of a complex read-only class (game state)

1 Answers1