Initial starting player - v2

Question

This is the updated code using all suggestions from the previous question that I made here.

I have revised the naming of the variables, and i have included the classes that this part of code is using, in order to have a better understanding.

I'm making a game battle system, where there are 2 players.

It is like a round based game.

• Every player can have multiple units to control, but can only use one of them each round..
• Every unit has an int CurrentSpeed variable;

I need to have in 2 variables who attacks first (Player firstPlayer) and who attacks second (Player secondPlayer), based on the speed of the active Unit.

At the moment I have this code:

public class Player {
 public int Id { get; set; }
 public string Name { get; set; }
 public List<Unit> Party { get; set; }
 public Unit Active { get; set; }
}
public class Unit {
 public string Name;
 public int InitialSpeed { get; set; }
 public int CurrentSpeed { get; set; }
}
public class BattleSystem {
 private void BattleTurn(){
 Player firstPlayer = null; //Player attacking first
 Player secondPlayer = null; //Player attacking second
 //Check the speed of the units, and determine who attack first
 firstPlayer = player_1.Active.CurrentSpeed > player_2.Active.CurrentSpeed ? player_1 : player_1.Active.CurrentSpeed < player_2.Active.CurrentSpeed ? player_2 : null;
 //if firstPlayer is null (active units have the same CurrentSpeed)
 if(firstPlayer == null){
 //Randomize who goes first
 System.Random rnd = new System.Random();
 int rng = rnd.Next(2);
 firstPlayer = rng == 0 ? player_1 : player_2;
 secondPlayer = rng == 0 ? player_2 : player_1;
 }else{
 secondPlayer = firstPlayer.Id == player_1.Id ? player_2 : player_1;
 }
 }
}

The part of determine the firstPlayer is ok, but I don't like the checks and assignments of the secondPlayer, like checking the ids.

I need tips on how I can code this in a better way.

Answer 1

Do without the ternary expressions: because a ternary expression is very good for initializing one variable, but is much less good for initializing two variables.

// Declared but not initialized (to null), in order to guarantee that
// they are each initialized in the subsequent if statements.
Player firstPlayer;
Player secondPlayer;
if (player_1.Active.CurrentSpeed > player_2.Active.CurrentSpeed)
{
 firstPlayer = player_1;
 secondPlayer = player_2;
}
else if (player_1.Active.CurrentSpeed < player_2.Active.CurrentSpeed)
{
 firstPlayer = player_2;
 secondPlayer = player_1;
}
else
{
 //Randomize who goes first
 ... etc ...
}

Answer 2

In addition to ChrisW's answer:

• There is little point in recreating Random variable every time you need it. You should consider saving it to the private field.
• You should also probably switch to C# coding style from what seems to be a Java style when it comes to using braces. Check ChrisW's example - opening braces go to the new line. Same goes for operators such as else.
• As a rule of a thumb - you should not expose public fields (unless you have a reason to). So Unit.Name should probably be an auto-property as well.
• It is a good idea to always have Id set accesor private.

• svick: "There is little point in recreating Random" More than that: it's wrong, as I explain in my answer to the previous question.
• Fr0z3n: For the brackets, for me it is really a personal coding style. I always coded in that way, so for me it is more readable. Can you please explain better you 3rd point? I do set public fields because i do have to set the values when i initialize a new instance of the class. If i set them private i cannot access them outside of that class. (I'm still learning, so I'm probably doing it wrong :P) Thanks.
• ChrisW: @WiS3 If you use a Microsoft IDE for editing, the braces snap to separate lines as you edit the file (unless you override the default formatting options).
• Fr0z3n: @svick I do use MonoDevelop mainly (because it integrate very well with Unity Engine).
• Nikita B: @WiS3, I do understand that. Nonetheless, you will eventually work in a team, and you will have to play by the rules. So it is good idea to keep your mind flexible right know and learn the ability to change your coding style depending on language. The longer you keep this "i like it that way"-mindset, the harder it will be to drop this habit later on. :)