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

Football Data Design

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

Problem

I'm working with the XMLSOCCER.COM API. I'm a little confused with how to best design the application. The problem I'm facing is that teams do not have a league ID so I couldn't associate a team with a league by that.

In order to associate them my league has a List and Team has List.

I'm trying to reduce the number of calls to the API but I can't find a way around this.

Classes

```
class Leagues
{

public List _Leagues { get; set; }
public Leagues()
{

XmlDocument xdoc = Program.MakeRequest("http://www.xmlsoccer.com/FootballDataDemo.asmx/GetAllLeagues", Program.API_KEY);

StringBuilder output = new StringBuilder();

string json = JsonConvert.SerializeXmlNode(xdoc);

JObject o = JObject.Parse(json);

IList results = o["XMLSOCCER.COM"]["League"].Children().ToList();

IList leagues = new List();

_Leagues = new List();
foreach (JToken result in results)
{
League league = JsonConvert.DeserializeObject(result.ToString());
_Leagues.Add(league);
}

}
}
class League
{
public int id { get; set; }
public string Name { get; set; }
public string Country { get; set; }
public string Historical_Data { get; set; }
public string Fixtures { get; set; }
public string Livescore { get; set; }
public int NumberOfMatches { get; set; }
public DateTime LatestMatch { get; set; }

public List Teams { get; set; }

public League()
{

}

public void GetLeagueTeamsByID(int league, string seasondateString){
var url = String.Format("http://www.xmlsoccer.com/FootballDataDemo.asmx/GetAllTeamsByLeagueAndSeason?ApiKey={0}&league={1}&seasonDateString={2}", Program.API_KEY, id, seasondateString);

try
{

XmlDocument xdoc = Program.MakeRequest(url

Solution

What @RubberDuck said

plus... I want to be clear that your Leagues / Teams / Players collections etc. - your domain / business objects, should work amongst themselves only. That DAL will create these. Then methods like FindByTeamName() are trivial because we're past all that XML stuff.

public class League {
    protected List Teams { get; set; }

    ///
    /// Case Sensitive search by Name. Returns null if not found
    /// 
    public Team FindByName(string teamName) {
        return Teams.Find(x -> x.Name == teamName);
    }
}


I'm trying to reduce the number of calls to the API but I can't find a way around this.

I think the main problem is that everything is public. So client code can do anything anywhere at any time - a free-for-all of code calls.

The problem I'm facing is that teams do not have a league ID so I couldn't associate a team with a league by that.

In order to associate them my league has a List and Team has List.

Do you mean that if you had that ID then a League would not have a List? But of course it should! You are not building a normalized relational database here.

@RubberDuck said: "I really don't like that there's sooo much logic in your constructors. "

I want to emphasize that you should not dump raw XML into your constructors and parse that out - I mean for example a single team's XML into the Team constructor. Instead your "DAL/Factory" should parse it all and:

  • create Player objects



  • create a List



  • create a Team and pass that List in the constructor



  • create a List



  • create a League and pass List in the constructor.



Well, you can certainly have Team.Add(Player aPlayer), League.Add(Team aTeam), etc. to build up a Team one player at a time, for example - as the DAL/factory builds them.

... I have to call the GetTeamsByLeagueAndSeason in the league construction and vice versa for teams -> players.

No. No you don't. You'll have methods for that. Called after everything is constructed.

public class Leagues {
    protected List Leagues { get; set; }
    
    public ??? GetLeagueBySeason (string LeagueName, string season) {
        League thisLeague = GetByName (sring LeagueName);
        
        if(thisLeague != null)
            thisLeague.GetSeason(season);
    }
    
    public League GetByName(string leagueName) {
        return Leagues.Find(x -> x.Name == leagueName);
    }
}

public class League {
    protected List Teams { get; set; }
    
    public ????? GetSeason (string thisSeason){
        // I have no idea what a season is. I expect you need
        // a Season class. Perhaps it has a complete list of games,
        // which is yet another class. A Game: date, opponents, score.
        // then a season: year, List
    }
}

Code Snippets

public class League {
    protected List<Team> Teams { get; set; }

    ///<summary>
    /// Case Sensitive search by Name. Returns null if not found
    ///<summary> 
    public Team FindByName(string teamName) {
        return Teams.Find(x -> x.Name == teamName);
    }
}
public class Leagues {
    protected List<League> Leagues { get; set; }
    
    public ??? GetLeagueBySeason (string LeagueName, string season) {
        League thisLeague = GetByName (sring LeagueName);
        
        if(thisLeague != null)
            thisLeague.GetSeason(season);
    }
    
    public League GetByName(string leagueName) {
        return Leagues.Find(x -> x.Name == leagueName);
    }
}

public class League {
    protected List<Team> Teams { get; set; }
    
    public ????? GetSeason (string thisSeason){
        // I have no idea what a season is. I expect you need
        // a Season class. Perhaps it has a complete list of games,
        // which is yet another class. A Game: date, opponents, score.
        // then a season: year, List<Game>
    }
}

Context

StackExchange Code Review Q#88637, answer score: 5

Revisions (0)

No revisions yet.