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 ?

asked Jul 4, 2016 at 13:52
\$\endgroup\$

1 Answer 1

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();
}
Heslacher
50.9k5 gold badges83 silver badges177 bronze badges
answered Jul 4, 2016 at 17:09
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.