Perfect time to introduce exceptions

[srgfraser]

In the code GetStat(), you added checks for areas which have errors due to programmer mistakes and when in production should never occur. Because of this, I believe, it is the correct time to use exceptions, instead of boundary checks. So the code should read:

        public float GetStat(Stat stat, CharacterClass characterClass, int level)
        {
            try
            {
                BuildLookupTable();
                return lookupTable[characterClass][stat][level-1];
            }
            catch (Exception) // should never get here
            {
                return 0;
            }
        }

I would also suggest logging the error, so that it can be found quicker.

[Brian_Trotter]

Logging is a great idea, and I’m also a fan of try/catch blocks.
A word of caution in this case… What we’re catching in our GetStat code are not programmer mistakes, but designer mistakes. Specifically, we’re catching things that weren’t properly set up in the Progression file.
The Exception will happily tell you that it’s an out of bounds error, but it won’t say what was missing, so logging it won’t be terribly specific. Testing each phase gives us the opportunity to log out the element that is missing in the Progression file…

For example: My GetStat looks like this:

if(!lookupTable.ContainsKey(characterClass)
{
    Debug.Log($"{name} does not contain an entry for characterClass");
    return 0;
}
if(!lookupTable[characterClass].ContainsKey(stat))
{
    Debug.Log($"{name} does not contain an entry for {stat} in the class {characterClass}");
    return 0;
}
return lookupTable[characterClass][stat][Mathf.Min(level, lookupTable[characterClass][stat].Length-1)];

It is a bit more code and cumbersome, but it does tell me with pinpoint accuracy when I’m missing an entry.

[srgfraser]

You are 100% correct this would be a designer’s mistake… Lol in my case developer and designer are the same person . Come to think of it… is it actually a designer’s mistake or a content producer’s mistake? (I’m sure there is an actual title for that).

Hmm… would this be better? It avoids all the if statements unless they are needed and removes the need for the Mathf logic. Notice the last log statement also pin points the missing level (I did not log ‘level - 1’ as the designer only cares about ‘level’).

try
{
    BuildLookupTable();
    return lookupTable[characterClass][stat][level-1];
}
catch (Exception) // should never get here
{
    if(!lookupTable.ContainsKey(characterClass)
    {
        Debug.Log($"{name} does not contain an entry for characterClass");
    }
    else if(!lookupTable[characterClass].ContainsKey(stat))
    {
        Debug.Log($"{name} does not contain an entry for {stat} in the class {characterClass}");
    }
    else
    {
        Debug.Log($"{name} does not nave a level {level} for {stat} in the class {characterClass}");
    }
    return 0;
}

[Brian_Trotter]

Yes, that would absolutely work. The MathF logic is to always return the last value regardless of the the level if the level is exceeded. That way if the character is level 6, but there are only attack values to level 5, the level 5 attack value will be returned. Logging it out when this is the case, however, should always be the case, so I would simply follow up that last Debug.Log with

return lookupTable[characterClass][stat][lookupTable[characterClass][stat].Length-1];

This is a personal choice, but it also avoids very confusing NaN issues on health bars.

[James_Lafritz]

Me personally I prefer throwing an Argument Out Of Range Exception and dealing with it in the calling class or allowing the Game to stop right there so the issue can be fixed right away. In my case when working on this I used a try catch block in the Base Stats and just logged a warning that I did not set the required Class Stats as I knew that while working on this portion of the course I would have things that I did not fill out in progression yet.

Or at the very least Log Warnings and Errors instead of just a regular Informative message.

try
{
    BuildLookupTable();
    return lookupTable[characterClass][stat][level-1];
}
catch (Exception e) // should never get here
{
    if(!lookupTable.ContainsKey(characterClass)
    {
        throw new System.ArgumentOutOfRangeException(nameof(characterClasses),
        //Debug.LogError(
                            $"{name} does not contain an entry for characterClass");
        //return 0;
    }
    else if(!lookupTable[characterClass].ContainsKey(stat))
    {
        throw new System.ArgumentOutOfRangeException(nameof(characterClasses),
        //Debug.LogError(
                            $"{name} does not contain an entry for {stat} in the class {characterClass}");
        //return 0;
    }
    else if (lookupTable[characterClass][stat].Length < level)
    {
        Debug.LogWarning($"{name} does not nave a level {level} for {stat} in the class {characterClass}");
        return lookupTable[characterClass][stat][lookupTable[characterClass][stat].Length-1];
    }

    Debug.LogError("Something went Horribly wrong!\n{e}")
    return 0;
}

I do this because I use the Log window to make sure that things are working the way I expect them to be while Debugging/Testing.