Class Design Question: Union of List<ChildStat> and AllStats

I have a Player class and a Stat class. The Player class has a List property where PlayerStat has a List and XP properties. I think my design is flawed because I am having trouble doing things that I think should be easy. I want to list out all Players and their XP for all Stats. Below are more details of the classes and the GetCompletePlayerStats method which is what I really don't like. I basically need to list out the XP for all stats for a Player, if the player doesn't have a stat then it should have an XP of zero. Any design help/suggestions would be appreciated.

public class Stat : EntityBase{
  public virtual string Name { get; set; }
  public virtual UnitOfMeasure Unit { get; set; }
  public virtual int UnitXP { get; set; }
}
public class Player : EntityBase{
  public virtual string Name { get; set; }
  public virtual IList<PlayerStat> PlayerStats { get; set; }

  public virtual List<PlayerStat> GetCompletePlayerStats(IQueryable<Stat> stats)
    {
        var allStats = new List<PlayerStat>();
        var playerStatIds = PlayerStats.Select(ps => ps.PlayerStatistic.Id).ToList();
        if (playerStatIds.Count == 0)
        {
            allStats.AddRange(stats.Select(stat => new PlayerStat() {PlayerStatistic = stat, XP = 0}));
        }
        else
        {
            var zeroStats = stats.Where(s => !playerStatIds.Contains(s.Id)).ToList();

            al开发者_Python百科lStats.AddRange(zeroStats.Select(zeroStat => new PlayerStat() {PlayerStatistic = zeroStat, XP = 0}));
            allStats.AddRange(PlayerStats);
        }

        return allStats;
    }

}
public class PlayerStat : EntityBase{
  public virtual Stat PlayerStatistic { get; set; }
  public virtual double XP { get; set; }
}

I have to admit, I dont really see a major drawback in your class design so far. Of course I dont have any insight in the greater picture and how your game is designed, since this is only a little section of it.

However, you said it is the GetCompletePlayerStats that you dont really like. I had to read it several times to understand what you are trying to do here. If I saw that right, you just want to return a PlayerStat object corresponding to each given Stat object. I guess Stat has an Id field (you are using it in your method) to compare two of them for semantic equality.

Given, that I made the right assumptions so far (unfortunately, you didnt provide much info), the method can be simplified to something like:

public virtual IEnumerable<PlayerStat> GetCompletePlayerStats(IEnumerable<Stat> stats)
{
    foreach(Stat stat in stats)
        yield return PlayerStats.FirstOrDefault(s => stat.Id == s.PlayerStatistic.Id) ?? 
                     new PlayerStat() {PlayerStatistic = stat, XP = 0};
    yield break;
}

This method here doesnt require a IQueryable but rather a IEnumerable to iterate over via a foreach loop, and yield out the corresponding PlayerStats object if there is one, or create a new one with XP set to 0 otherwise (The null-coalescing operator ?? is very useful in those cases).

Hope that helps!

With the existing design, this method can be simplified thus:

public virtual IList<PlayerStat> GetCompletePlayerStats(IEnumerable<Stat> stats)
{
   // build a dictionary of existing stats by ID to facilitate 
   // the join with requested stats (effectively doing a hash join)
   var playerStatsById = PlayerStats.ToDictionary(ps => ps.PlayerStatistic.Id);

   // for each requested stat, return either the corresponding player stat 
   // or a zero stat if one isn't found, maintaining the original order of stats
   return stats
       .Select(s => playerStatsById.ContainsKey(s.Id) ? 
           playerStatsById[s.Id] : 
           new PlayerStat { PlayerStatistic = s, XP = 0 })
       .ToList();
}

Note that since this is effectively an outer-join operation (you're joining stats with PlayerStats but you also need the non-matches to yield as zero) you can also do it with LINQ's GroupJoin() operation, although I suspect that would be much less readable.

What does bother me about your design is the ambiguity of names, for example you have both the PlayerStat class and the PlayerStatistic property and they mean subtly different things; consider naming things differently and things might look better; in fact that's probably the best design advice I can give you, for any situation :)