Tracking sports statistics

Question

I am designing a new application to track sports statistics with code-first migration. I have the bare minimum POCO setup. The problem is that I am not happy with the design specifically how the players, teams and tournaments are relating to each other.

I appreciate any comments and suggestions.

PlayerAccount class:

/// <summary>
/// Player account is a separate entity from the actual user account.
/// Player account allows a user to join teams and participate in a tournament.
/// </summary>
public class PlayerAccount
{
    [Key]
    public int Id { get; set; }
    [Required]
    public string FirstName { get; set; }
    [Required]
    public string LastName { get; set; }

    /// <summary>
    /// TournamentPlayerKeys tells us which team this player is competing with 
    /// in a certain tournament.
    /// </summary>
    public virtual ICollection<PlayerTeamKey> PlayerTeamKeys { get; set; }
    public virtual ICollection<PlayerMatchStat> PlayerMatchStats { get; set; }
}

Team class:

using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;

/// <summary>
/// Team can be formed by a single user or a group of user. You'll need a team to join a tournament.
/// A single user can form his own team. 
/// </summary>
public class Team
{
    [Key]
    public int Id { get; set; }
    [Required]
    public string Name { get; set; }

    public virtual ICollection<PlayerTeamKey> PlayerTeamKeys { get; set; }
    public virtual ICollection<TeamTournamentKey> TeamTournamentKeys { get; set; }
}

Tournament class:

using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;

/// <summary>
/// Tournaments are basically a collection of games in one event. 
/// For example, Summer 2014 Basketball League.
/// </summary>
public class Tournament
{
    [Key]
    public int Id { get; set; }
    [Required]
    public string Name { get; set; }

    public virtual ICollection<TeamTournamentKey> TeamTournamentKeys { get; set; }
    public virtual ICollection<TournamentMatch> TournamentMatches { get; set; }
}

On a tournament there are multiple instances of matches (games):

/// <summary>
/// Match represents one occurence of a game in a league.
/// </summary>
public class TournamentMatch
{
    [Key]
    public int Id { get; set; }
    [Required]
    public int Tournament_Id { get; set; }

    [ForeignKey("Tournament_Id")]
    public virtual Tournament Tournament { get; set; }
    public virtual ICollection<TournamentMatchResult> TournamentMatchResults { get; set; }
}

And of course we have multiple instances of match results in one match. The Score property determines who is the winner.

/// <summary>
/// This is the result of a league match.
/// </summary>
public class TournamentMatchResult
{
    [Key]
    public int Id { get; set; }
    [Required]
    public int Match_Id { get; set; }
    [Required]
    public int Team_Id { get; set; }
    [Required]
    public double Score { get; set; }

    [ForeignKey("Match_Id")]
    public virtual TournamentMatch TournamentMatch { get; set; }
    [ForeignKey("Team_Id")]
    public virtual Team Team { get; set; }
    public virtual ICollection<PlayerMatchStat> PlayerMatchStat { get; set; }

}

I have a multipart key that creates the relationship between a player and a team:

/// <summary>
/// This is a multipart key class that defines a player as part of a team.
/// Mind the naming convension, from smaller entity to a bigger entity; Player > Team.
/// </summary>
public class PlayerTeamKey
{

    [Required]
    public int PlayerAccount_Id { get; set; }
    [Required]
    public int Team_Id { get; set; }

    [ForeignKey("PlayerAccount_Id")]
    public virtual PlayerAccount Player { get; set; }
    [ForeignKey("Team_Id")]
    public virtual Team Team { get; set; }

}

And I have another like that for team and tournament:

/// <summary>
/// This is a multipart key class that is a combination of a team id and a tournament id.
/// This basically tells us that the team with the given id is going to participating on a tournament with the given id.
/// </summary>
public class TeamTournamentKey
{
    [Required]
    public int Team_Id { get; set; }
    [Required]
    public int Tournament_Id { get; set; }

    [ForeignKey("Team_Id")]
    public virtual Team Team { get; set; }
    [ForeignKey("Tournament_Id")]
    public virtual Tournament Tournament { get; set; }

}

Answer 1

Don't use underscores in identifiers. If that's how the database has them, use a ColumnAttribute to specify the column name scratch that, you're going code-first. This:

public int Tournament_Id { get; set; }

public int Match_Id { get; set; }
public int Team_Id { get; set; }

public int PlayerAccount_Id { get; set; }

Is much more seesharpesque like this:

public int TournamentId { get; set; }

public int MatchId { get; set; }
public int TeamId { get; set; }

public int PlayerAccountId { get; set; }

I like that most of these foreign key properties follow a very predictable naming pattern [ForeignEntityTypeName]Id - IIRC that's all EF needs to be able to infer the foreign keys.

...With the exception of TournamentMatchResult, which breaks the pattern with its MatchId (/Match_Id) property:

public int Match_Id { get; set; }
public virtual TournamentMatch TournamentMatch { get; set; }

Would be more consistent like this:

public int TournamentMatchId { get; set; }
public virtual TournamentMatch TournamentMatch { get; set; }

---

You're using System.ComponentModel.DataAnnotations;; it works, but I find all these attributes pollute the POCO:

[Key]
public int Id { get; set; }
[Required]
public int Match_Id { get; set; }
[Required]
public int Team_Id { get; set; }
[Required]
public double Score { get; set; }

[ForeignKey("Match_Id")]
public virtual TournamentMatch TournamentMatch { get; set; }
[ForeignKey("Team_Id")]
public virtual Team Team { get; set; }

EF is smart enough to know Id is your [Key] (by naming convention), and smart enough to know Team is a FK that uses TeamId (by naming convention) - I'm not sure whether Team_Id breaks it or not, but relying on EF's established conventions makes your POCO code less polluted:

public int Id { get; set; }

[Required]
public int TournamentMatchId { get; set; }

[Required]
public int TeamId { get; set; }

[Required]
public double Score { get; set; }

public virtual TournamentMatch TournamentMatch { get; set; }
public virtual Team Team { get; set; }

Now all that's left is [Required] attributes. You could remove them too, if you moved POCO configurations/mappings elsewhere.

---

One way of doing that would be in the OnModelCreating override of your DbContext:

modelBuilder.Entity<TournamentMatchResult>()
            .Property(t => t.TournamentMatchId).IsRequired();
modelBuilder.Entity<TournamentMatchResult>()
            .Property(t => t.TeamId).IsRequired();
modelBuilder.Entity<TournamentMatchResult>()
            .Property(t => t.Score).IsRequired();

Obviously this gets very verbose, very fast. An alternative is to use EntityMappingConfiguration<TEntityType> classes. For each entity type, you create a mapping class:

public class TournamentMatchResultMap : EntityMappingConfiguration<TournamentMatchResult>
{
    public TournamentMatchResultMap()
    {
        Property(t => t.TournamentMatchId).IsRequired();
        Property(t => t.TeamId).IsRequired();
        Property(t => t.Score).IsRequired();
    }
}

You can also use these mapping classes to configure mapped column names and table, and relationships with other entities. See EntityMappingConfiguration<TEntityType>

Then the OnModelCreating override simply loads all the configurations:

modelBuilder.Configurations.Add(new TournamentMatchResultMap());

I find that's the cleanest approach; it lets your POCO classes be POCO's, it leaves the OnModelCreating method free of verbose fluent API mappings, and it puts each entity's configurations into its own class.

---

Let's look at the relationships. If I didn't make mistakes, this is what your code can be modeled as:

As @Jeroen explains, EF doesn't need junction tables to model a many-to-many relationship; PlayerTeamKey and TeamTournamentKey entities can be removed.

I would believe this simplified model could very well suit your needs:

The idea is to use the navigation properties to navigate between entities - from a business standpoint it makes sense for tournament matches to know what teams are involved, but from a data standpoint, it doesn't: when your application creates a new tournament, it also creates a TournamentMatchResult for all teams involved, with each team's score at 0 - and that's enough for a TournamentMatch to know what teams are playing.

As the game evolves, you can then update the TournamentMatchResult records, and if a TournamentMatch needs to know what teams are involved, it can easily find out by selecting its .TournamentMatchResults.Select(result => result.Team).

Notice this results in simple, easy-to-follow, one-to-many relationships everywhere.

Answer 2

I made a small example to show you that all your worries are in the past. Before you implement this yourself though, keep Mat's remarks in mind: I made this as a quick sketch and separating the mappings from the context, naming conventions, etc are important for your code's clarity.

That being said, this small setup shows you how you can change it:

public class Player
{
    public int Id { get; set; }
    public string Firstname { get; set; }
    public string Lastname { get; set; }

    public virtual ICollection<Team> Teams { get; set; }
}

public class Team
{
    public int Id { get; set; }
    public string Name { get; set; }

    public virtual ICollection<Player> Players { get; set; }
}

That's it. You're done! All we need to do now is configure our mapping so it will generate the intermediate table itself:

public class TournamentContext : DbContext
{
    public DbSet<Player> Players { get; set; }
    public DbSet<Team> Teams { get; set; }

    protected override void OnModelCreating(DbModelBuilder modelBuilder)
    {
        modelBuilder.Entity<Player>()
            .HasKey(x => x.Id)
            .HasMany(x => x.Teams);

        modelBuilder.Entity<Team>()
            .HasKey(x => x.Id)
            .HasMany(x => x.Players);
    } 
}

If you want to test it out yourself, you can use this sample code:

static void Main(string[] args)
{
  using (var context = new TournamentContext())
  {
      context.Database.ExecuteSqlCommand("delete from Players");
      context.Database.ExecuteSqlCommand("delete from Teams");

      var team1 = context.Teams.Add(new Team {Name = "Power rangers"});
      var team2 = context.Teams.Add(new Team { Name = "Ninja turtles" });

      context.Players.Add(new Player
      {
          Firstname = "John",
          Lastname = "Johnson",
          Teams = new List<Team> {team1, team2}
      });

      context.Players.Add(new Player
      {
          Firstname = "Jack",
          Lastname = "Jackson",
          Teams = new List<Team>{team1}
      });

      context.SaveChanges();

      foreach (var player in context.Players.ToList())
      {
          Console.WriteLine("Player: " + player.Id);
          Console.WriteLine("Teams: " + string.Join(", ", player.Teams.Select(x => x.Name).ToArray()));
      }
  }

  Console.Read();
}

Looking at our database using the Server Explorer we now see these tables:

With this output:

You'll find that you can configure this a lot and add restrictions to the table that gets generated but I'm too lazy to look them up myself; mapping isn't my best side. This should give you an idea though about how you can get rid of those not-so-pretty intermediate tables.

Answer 3

I suggest to use the same name for primary and foreign keys following some consistent pattern:

It could be e.g. _id or Id, so you will have in PlayerAccount table the primary key PlayerAccountId (instead of just Id) and in PlayerTeamKey also PlayerAccountId as a foreign key.

Naming of fields and classes is very important in non-small projects.

I believe it does not matter whether you follow code-first or model-first or whatever approach, your problem is propper class design and naming. I personally like very much code-second pattern using PowerToys - you can do your data diagram 1] in a object design tool like Enterprise Architect 2] in SQL database like SQL Server. Then you export this design as code-first using PowerToys.

You must organize the terms you use and convert them to meaningful class design, which is not allways easy.

Here is the diagram of classes you presented:

You can create quite easily this class diagram in Visual Studio. Appearently there is missing Match class. (Match_Id - foreign key) (OK it is not missing, it is a link to the very suspicious TournamentMatch class, I get it)

Your basic classes are Player (originally PlayerAccount), Team, Match (originally TournamentMatchResult), Tournament. I have renamed the classes - it is more comprehensible this way I think. The other classes are empty association classes that are used to create M:N relation. Are all those M:N relations really necessary and OK?

There are too many unanswered questions

Basically your main problem is that in your design you have too many atribute-less association classes (PlayerTeamKey, TournamentMatch ... I see 4 of them), which is very probably error in class design.

You should first answer the kind of questions, which I wrote into second diagram, and then and only then start writing your code. Otherwise your design will fail.

EDIT:

To give an example how you can make proper design by answering analytical questions, I have created simplified version. You can as I believe, create your model with just 4 classes depending on how you answer the analytical questions. This model can be easily convereted to C# code first, but firts confirm whether this is the model you seek.

One more hint: if you use word Match for a match, do not call it Game, use just as few terms as it is absolutely necessary.