HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpModerate

Tracking sports statistics

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
trackingstatisticssports

Problem

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:

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

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


Team class:

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

/// 
/// 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. 
/// 
public class Team
{
    [Key]
    public int Id { get; set; }
    [Required]
    public string Name { get; set; }

    public virtual ICollection PlayerTeamKeys { get; set; }
    public virtual ICollection TeamTournamentKeys { get; set; }
}


Tournament class:

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

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

    public virtual ICollection TeamTournamentKeys { get; set; }
    public virtual ICollection TournamentMatches { get; set; }
}


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

```
///
/// Match repre

Solution

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()
            .Property(t => t.TournamentMatchId).IsRequired();
modelBuilder.Entity()
            .Property(t => t.TeamId).IsRequired();
modelBuilder.Entity()
            .Property(t => t.Score).IsRequired();


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

public class TournamentMatchResultMap : EntityMappingConfiguration
{
    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

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.

Code Snippets

public int Tournament_Id { get; set; }

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

public int PlayerAccount_Id { get; set; }
public int TournamentId { get; set; }

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

public int PlayerAccountId { get; set; }
public int Match_Id { get; set; }
public virtual TournamentMatch TournamentMatch { get; set; }
public int TournamentMatchId { get; set; }
public virtual TournamentMatch TournamentMatch { get; set; }
[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; }

Context

StackExchange Code Review Q#43762, answer score: 10

Revisions (0)

No revisions yet.