Mapping enum keys to instance values

Question

I am working on a pathfinding program / algorithm, and I have the following class:

[System.Serializable]
public class UnitTileCosts
{
    int[] tileCosts;

    public UnitTileCosts()
    {
        //empty constructor uses default value of 1 for all fields
        GenerateDefaults();
    }

    public UnitTileCosts(Dictionary<TileType,int> costs)
    {
        GenerateDefaults();
        foreach(var pair in costs)
        {
            tileCosts[(int) pair.Key] = pair.Value;
        }
    }

    void GenerateDefaults()
    {
        tileCosts = new int[System.Enum.GetNames(typeof(TileType)).Length];
        for(int i = 0; i < tileCosts.Length; i++)
        {
            tileCosts[i] = 1;
        }
    }

    public int GetTileCost(TileType tile)
    {
        return tileCosts[(int) tile];
    }
}

Is this better or worse than simply using a Dictionary<TileType, int> in the first place?

Using the Dictionary (that is passed in to the constructor), I would need to check if the key exists because I only care about those values that differ from the result. (Which, admittedly, is easily doable with TryGetValue and an OUT parameter)

I would think that an array of ints would serialize better than a dictionary, to boot.

Is casting from the enum to the int acceptable in this instance? I understand that enums are designed to avoid magic numbers, but in this case I only care about the numbers (and not the magic), so I think it might be reasonable.

Answer 1

I'm not sure if you're gaining anything from this wrapper class or not, but there is an alternative for initializing an array to have all the same value at each position. You could use Enumerable.Repeat instead of the loop.

void GenerateDefaults()
{
    tileCosts = Enumerable.Repeat(1, System.Enum.GetNames(typeof(TileType)).Length).ToArray();
}

But perhaps the one liner is a bit too terse to be readable.

void GenerateDefaults()
{
    var length = System.Enum.GetNames(typeof(TileType)).Length;

    tileCosts = Enumerable.Repeat(1, length).ToArray();
}

Whether or not that's an improvement may be debatable and is mostly a matter of preference in my opinion.

Something else I don't care for is the fact that GenerateDefaults is a void. As such, it's not immediately clear that it modifies the tileCosts class variable. I would redefine it to return int[]. It's a few more keystrokes to call it, but it becomes obvious where tileCosts is being modified.

[System.Serializable]
public class UnitTileCosts
{
    int[] tileCosts;

    public UnitTileCosts()
    {
        //empty constructor uses default value of 1 for all fields
        tileCosts = GenerateDefaults();
    }

    public UnitTileCosts(Dictionary<TileType,int> costs)
    {
        tileCosts = GenerateDefaults();
        foreach(var pair in costs)
        {
            tileCosts[(int) pair.Key] = pair.Value;
        }
    }

Which begs the question, why set a default value at all if you're just going to overwrite the value a split second later?? It doesn't make sense to me and I would remove that code. At the least, I would make the overloaded constructor call the default constructor to dry things up.

    public UnitTileCosts(Dictionary<TileType,int> costs) : this()
    {
        foreach(var pair in costs)
        {
            tileCosts[(int) pair.Key] = pair.Value;
        }
    }   

Answer 2

I could be way off base here, since I don't know the rest of your design, but have you considered changing the enum into a class?

public class UnitTile
{
    public UnitTile(int tileType)
    {
        TileType = tileType;
        Cost = 1;
    }

    public int TileType { get; private set; }
    public int Cost { get; set; }
}

This way you could avoid classes like UnitTileCosts altogether and just generate your various tile types and costs in one place. Perhaps you still store the various tile types in a Dictionary or array, but at least you'd have the ability to add extra properties to them later.