2
\$\begingroup\$

I'm developing a game in Unity 5.3.5, I have a player statistics system with 10 different stats, as in probably every game the user is able to see the points put in each stats and also the benefit each stats is giving him. I have 2 panels with 2 text the first one is responsible for the points in each stats and the second one for the increase each stats is giving. The player has a class which holds all the stats in there and also implements <IEnumberable> so I can be able to iterate over this class picking all the different stats, I also have an enum which is used only for the displaying of those statistics and it's in such order that it matches the player's class that holds all the stats. Only 1 panel can be active at a time. That's how I'm updating the statistics info :

    private void UpdateStatusLabels(PanelType panelType)
    {
        List<Stats.Stats> playerStats = Player.BaseStats.ToList();
        string playerStatsText = string.Empty;
        Array values = Enum.GetValues(typeof(StatsOrders));
        string[] names = (from object value in values select value.ToString()).ToArray();
        for (int i = 0; i < names.Length; i++)
        {
            playerStatsText += panelType == PanelType.Stats
                ? names[i].SplitOnCapitalLetters() + @" : " + playerStats[i].Increase(playerStats[i].PointsInStats)
                : names[i].SplitOnCapitalLetters() + @" : " + playerStats[i].PointsInStats;
            playerStatsText += Environment.NewLine;
        }
        SetText(panelType == PanelType.Points ? panelPoints : panelStats, "EditorOnly", playerStatsText);
    }

    private static void SetText(GameObject inputGameObject, string textTag, string textToSet)
    {
        inputGameObject.GetComponentsInChildren<Text>().Single(comp => comp.tag == textTag).text = textToSet;
    }

here are some extra objects you might need :

public enum StatsOrders
{
    MagicPower,
    DebuffDuration,
    Defense,
    MagicDefense,
    CriticalPower,
    CriticalChance,
    Multistrike,
    CooldownReduction,
    Evasion,
    Resistance
}
private enum PanelType
{
    Stats,
    Points
}

private GameObject panelStats;
private GameObject panelPoints;

What concern's me is that it looks over-complicated, can the code be shorten or improved in some way ?

\$\endgroup\$
2
\$\begingroup\$

Your update status method does too much. It has to decide which panel to use, which string to create and even how to create it.

If you separated them you could focus on exacly one task at a time:

The entry method should just decide which specialized update method to use.

It passes the required parameters to them and lets them do the job.

After reciving the result it passes it to the SetText method.

private void UpdateStatusLabels(PanelType panelType)
{
    var playerStats = Player.BaseStats;     
    var values = Enum.GetValues(typeof(StatsOrders));
    var names = from object value in values select value.ToString();

    switch (panelType)
    {
        case PanelType.Stats:
            var playerStatsText = CreateStatsPanelText(playerStats, names);
            SetText(panelStats, "EditorOnly", playerStatsText);
        break;

        case PanelType.Points:
            // ...
        break;
    }

}

Everything the specialized methods do is to create the new string. (this is an example for one of them).

You don't need to use arrays and indexes. As you are looping over both collections synchronously you can make your life easier and use the Zip extension to iterate over both of them at the same time.

For creating the status text you should use the StringBuilder which makes the string creation cleaner.

private void CreateStatsPanelText(IEnumerable<Stats.Stats> playerStats, IEnumerable<string> names)
{
    var playerStatsText = new StringBuilder();

    foreach (playerStats.Zip(names, (playerStat, name) => new { playerStat, name }))
    {
        playerStatsText
            .Append(name.SplitOnCapitalLetters())
            .Append(" : ")
            .Append(playerStat.Increase(playerStat.PointsInStats))
            .AppendLine();
    }

    return playerStatsText.ToString();
}
| improve this answer | |
\$\endgroup\$

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.