3
\$\begingroup\$

I am writing a domino draw game in C# and would like to know if there is a simpler/better way to know the player that has been dealt the highest double bone (where x equals y) and what bone it is:

public class Player
{
    public string Name { get; set; }
    public List<Bone> Bones;
    public Player(string name)
    {
        Name = name;
    }
}

public class Bone 
{
    public int x { get; set; }
    public int y { get; set; }
}

Code in question (players is a list of player):

//select player with highest double and select the double
var pdb = players.SelectMany(pl => pl.Bones.Where(b => b.x == b.y)
    .Select(b => new { player = pl, doublebones = b }))
    .OrderByDescending(db => db.doublebones.x)
    .First();
Player playerWithHighestDouble = pdb.player;
Bone highestBone = pdb.doublebones;
\$\endgroup\$
4
\$\begingroup\$

Welcome to CodeReview. What you are trying to do is simple enough that your LINQ implementation is okay. Let's face it, with dominoes you are not going to have a million players with a million bones, so no need to worry about performance issues with a fairly small set.

You didn't ask explicitly for review of other parts of code, but I will offer them anyway.

Name could be a read-only property set only in the constructor.

You have Bones as a field, not a property. It could be exposed as a public IReadOnlyList, and it probably should be a property.

Bone could be a struct or a class since it only has 2 int properties. I do not like the names X and Y as I tend to think of Cartesian coordinates and therefore a location. I would prefer to see names like Square1 and Square2. I would probably keep this as a class and make the properties readonly as well. You may consider adding extra properties such as IsDouble, and even override ToString() with $"{Square1}-{Square2}", and maybe even Pips which is a sum of both squares.

As you are new to CR, since I have posted an answer, do NOT alter your code in your question or else a moderator will roll it back.

\$\endgroup\$
1
  • \$\begingroup\$ thanks a lot for the review, your suggestions make sense. \$\endgroup\$ – Alex Dec 12 '19 at 22:09

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.