Relationship between two interfaces and two classes.

Question

So I want to design a team/player relationship like this: every player belongs to one team but since I wanted to practice with interfaces I made ITeam and IAthlete and then made BasketballTeam and BasketballPlayer. Then I wrote this code:

public interface IAthlete
{
     string GetName();
     string GetSport(); 
}

public interface ITeam
{
     void AddPlayer(IAthlete player);
     IAthlete[] GetAthletes();
     string GetName();
     int GetNumberOfPlayers();
}

public class BasketballPlayer:IAthlete
{
    private string name; 

    public string GetName()
    {
        return this.name; 
    }

    public string GetSport()
    {
        return "Basketball"; 
    }

    public BasketballPlayer(string name)
    {
        this.name = name; 
    }

    public void Run(int distance)
    {
        Console.WriteLine(this.name + " just ran " + distance.ToString() + " meters."); 
    }

    public bool Shoot()
    {
        Console.WriteLine("Successful shot for " + this.name);
        return true; 
    }
}

public class BasketballTeam: ITeam      
{
    BasketballPlayer[] players;
    int numberOfPlayers; 
    private string name; 

    public void AddPlayer(BasketballPlayer player)
    {
        this.players[this.numberOfPlayers] = player;
        this.numberOfPlayers++; 
    }

    public IAthlete[] GetAthletes()
    {
        return this.players; 
    }

    public string GetName()
    {
        return this.name; 
    }

    public int GetNumberOfPlayers()
    {
        return this.numberOfPlayers; 
    }

    public BasketballTeam(string name)
    {
        this.numberOfPlayers = 0; 
        this.name = name;
        this.players = new BasketballPlayer[10]; 
    }
}

class Program
{
    static void Main(string[] args)
    {
        BasketballTeam bt = new BasketballTeam("MyTeam");
        BasketballPlayer bp = new BasketballPlayer("Bob");

        bt.AddPlayer(bp);

        foreach (BasketballPlayer player in bt.GetAthletes())
        {
            Console.WriteLine(player.GetName()); 
        }

        foreach (IAthlete a in bt.GetAthletes())
        {
            Console.WriteLine(a.GetName()); 
        }
    }
}

But it won't compile because I'm using this:

public void AddPlayer(BasketballPlayer player)

in the BasketballPlayer instead of this

public void AddPlayer(IAthlete player)

I thought it should work because BasketballPlayer is an IAthlete. And if I change it to IAthlete then I can make another class like this:

public class HockeyPlayer : IAthlete
{
    private string name;

    public string GetName()
    {
        return this.name;
    }

    public string GetSport()
    {
        return "Hockey";
    }

    public HockeyPlayer(string name)
    {
        this.name = name;
    }

    public void Run(int distance)
    {
        Console.WriteLine(this.name + " just ran " + distance.ToString() + " meters.");
    }
}

and then do this in my main:

 HockeyPlayer hp = new HockeyPlayer("Henry");
 bt.AddPlayer(hp);

which is logically wrong because I'm adding HockeyPlayer to a BasketballTeam. Is it supposed to be like this and I should just be careful not to do that? What am I doing wrong? How do I show this using class diagrams? Does this lead to loose coupling?

You should be using properties instead of most of those methods. — SLaks, Apr 09 '12 at 11:42

score 3 · Answer 1 · answered Apr 09 '12 at 11:39

3

You're trying to violate the Liskov Substitution Principle.

Anything that can be done with a supertype – such as adding a HockeyPlayer – can also be done with a subtype – including a BasketballTeam.

Instead, you should use generics:

class Team<TPlayer> where TPlayer : IAthlete {
    public ReadOnlyCollection<TPlayer> Players { get; }
    public string Name { get; }
    public void AddPlayer(TPlayer player);
}

answered Apr 09 '12 at 11:39

SLaks

868,454
176
1,908
1,964

The concept missing here is Game. – Aliostad Apr 09 '12 at 11:51
Thanks man didn't really think of LSP until you said it but I didn't quite get your code. I kind of a newbie. – arkazeminia Apr 09 '12 at 12:04

Sergey Berezovskiy · Accepted Answer · 2012-04-09T12:25:58.430

Here is some thoughts on your code. First, in C# you can use properties, instead of Get and Set methods.

public interface IAthlete
{
    string Name { get; }
    string Sport { get; }
}

With auto-properties you can ask compiler to generate back store for property. Also consider creating base class Player, which will hold implementation of Name and Sport properties.

public class Player : IAthlete
{
    public Player(string name, string sport)
    {
        Name = name;
        Sport = sport;
    }

    public string Name { get; private set; }
    public string Sport { get; private set; }        
}

Now when implementing some player, you can just pass values to base class constructor. And your custom players will hold only specific for them functionality (no code duplication). Also it's recommended to use string format, instead of concatenating strings:

public class BasketballPlayer : Player
{
    public BasketballPlayer(string name)
        : base(name, "Basketball")
    {
    }

    public void Run(int distance)
    {
        Console.WriteLine("{0} just ran {1} meters.", Name, distance);
    }

    public bool Shoot()
    {
        Console.WriteLine("Successful shot for " + Name);
        return true;
    }
}

Now about teams. If you don't want to have FootballPlayers in your BasketballTeam, then you should create parametrized team. Also consider using IEnumerable:

public interface ITeam<TPlayer>
    where TPlayer : IAthlete
{
    void AddPlayer(TPlayer player);
    IEnumerable<TPlayer> Players { get; }
    string Name { get; }
    int NumberOfPlayers { get; }
}

Again, for common functionality you can create base class. Keep in mind, that you should check how many players currently in your team before adding new player.

public class Team<TPlayer> : ITeam<TPlayer>
    where TPlayer : IAthlete
{
    private readonly List<TPlayer> _players = new List<TPlayer>();

    public Team(string name, int teamSize)
    {
        Name = name;
        TeamSize = teamSize;            
    }

    public void AddPlayer(TPlayer player)
    {
        if (_players.Count == TeamSize)
            throw new Exception("Players number exceeded");

        _players.Add(player);
    }

    public string Name { get; private set; }
    public int TeamSize { get; private set; }

    public IEnumerable<TPlayer> Players
    {
        get { return _players; }
    }

    public int NumberOfPlayers 
    {
        get { return _players.Count; }
    }       
}

And custom team implementation becomes really easy. You just tell which type of players it will have, and pass to base team implementation team name and size of team.

public class BasketballTeam : Team<BasketballPlayer>
{
    public BasketballTeam(string name)
        : base(name, 10)
    {
    }
}

Now your program works like a charm:

class Program
{
    static void Main(string[] args)
    {
        BasketballTeam bt = new BasketballTeam("MyTeam");
        BasketballPlayer bp = new BasketballPlayer("Bob");

        bt.AddPlayer(bp);

        foreach (BasketballPlayer player in bt.Players)
        {
            Console.WriteLine(player.Name);
        }

        foreach (IAthlete a in bt.Players)
        {
            Console.WriteLine(a.Name);
        }
    }
}

score 1 · Answer 3 · answered Apr 09 '12 at 13:27

Logically ,

These should be your base classes : Team , Player

These should be your derived classes : BasketballTeam , BasketballPalyer

These should be interfaces on Player : IPlay() , IRun , IGetName etc.. whichever applicable

and so on...

Guideline : Verbs suits more good on interfaces and Noun suits good on classes. Noun in the requirement best suits for Class in the code.

Aliostad · Answer 4 · 2012-04-09T11:51:59.850

If your interfaces are meant to be used by variety of games, it seems that you are missing the Game here and perhaps need to use Generics:

public interface IGame
{
   string Name {get;}
   ...
}

public class Bastketball : IGame
{
   ...
}

public interface ITeam<TGame> where TGame: class, IGame
{
   void AddPlayer(IPlayr<TGame> player);
   ...
}


public interface IPlayer<TGame> where TGame: class, IGame
{
   ...

}

This will prevent from hockey player to be added to Basketball team.

score 0 · Answer 5 · edited May 23 '17 at 12:11

SLaks is correct. You could add a generic constraint to your ITeam to not accept all players, but just those of one type:

public interface ITeam<T> where T : IAthlete
{
     void AddPlayer(T player);
     IAthlete[] GetAthletes();
     //  or: T[] GetAthletes();
     string GetName();
     int GetNumberOfPlayers();
}

A BasketballTeam implementation could look like:

public class BasketballTeam : ITeam<BasketballPlayer>
{
    BasketballPlayer[] players;
    // […]

    public void AddPlayer(BasketballPlayer player)
    {
        this.players[this.numberOfPlayers] = player;
        this.numberOfPlayers++;
    }

    public IAthlete[] GetAthletes()
    {
        return this.players; 
    }

    // or:
    // public BasketballPlayer[] GetAthletes()
    // {
    //     return this.players; 
    // }

    // […]
}

Thanks. I didn't understand what he said but I got what you explained. How do I show this in a class diagram? — arkazeminia, Apr 09 '12 at 12:03

Relationship between two interfaces and two classes.

5 Answers5