Resolved Question regarding the instantiation of object in class

[Matthieu]

Hi erveryone,

I got a question I want to clarify by using an example.

I have a Club class and a TennisPlayer class. A club can have many tennisplayers as members to that club, and a tennisplayer can only be member at one club at a time.
My classes would look like this. I simplified the classes for this example.

public class Club
    {
        private List<TennisPlayer> members;

        public Club()
        {
            members = new List<TennisPlayer>();
        }

        public void AddTennisPlayerAsMember(TennisPlayer tennisPlayer)
        {
            members.Add(tennisPlayer);
            tennisPlayer.Club = this;
        }
    }

public class TennisPlayer
    {
        public string Name;
        public Club Club;

        public TennisPlayer(string name, Club club)
        {
            Name = name;
            club.AddTennisPlayerAsMember(this);
        }
    }

So my question is, is it right that in the TennisPlayer class the field club has nothing like Club club = new Club()?
Cause if I would do that, every TennisPlayer object I instantiate would have his own Club object which is not logical because there is only one Club object for every club, just like there is in real life no duplicate of the same club, am I right?

 

[Skydiver]

My understanding from post #1 is that he wanted to model this relationship:

A TennisPlayer has 0 or 1 Club.
A Club has 0 or more TennisPlayer's.

In other words, a TennisPlayer can only belong to a single Club, and a Club can have multiple TennisPlayers. Furthermore, the TennisPlayer knows which Club he belongs to, and the Club knows which TennisPlayers are members.


The implementation #15 ends looking like this:

A WithMembersOfClubs has 0 or more Tuple<TennisPlayer, Club>.
A Tuple<TennisPlayer, Club> has 1 TennisPlayer and 1 Club.

The TennisPlayer now doesn't know which Club he/she belongs to, and the Club doesn't know which TennisPlayers are its members. Whomever is writing the code now has to consult a 3rd object: WithMembersOfClubs object to find out membership information. Furthermore, unless extra code is added into WithMembersOfClubs, the enforcement that a TennisPlayer can belong to at most one Club is missing.

This makes me sad for several reasons. First, it's suddenly more complex. Where we started off with two classes, now we are up to 4 classes. Second, we are now designing for a relational database right off the bat, rather than starting object oriented first. The first UML diagram above shows a nice purely object oriented view of things, but the second diagram is a concession to the current world we live in where a most people will end up persisting their objects into a relational database. If people used a real object oriented database, or had a very smart ORM (object to relational mapper), then there is no need for that Tuple that records the relationship between the TennisPlayer and the Club -- the (object oriented) database would just take care of it.

Some may argue that the first UML diagram is an oversimplication. It's hiding the list of TennisPlayer's used inside the Club. Okay, so here's a less simplified view showing that level of detail:

but what is good for the goose is also good for the gander, so let's also show that list of tuples inside the WithMembersOfClubs:

Anyway, here's the way I would have dealt with the TennisPlayer and Club:

using System;
using System.Collections;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;

class TennnisPlayer
{
    public string Name { get; set; }

    Club _club;
    public Club Club
    {
        get => _club;

        set
        {
            if (value != _club)
            {
                var oldClub = _club;
                _club = null;
                oldClub?.RemoveMember(this);
                _club = value;
                _club?.AddMember(this);
            }
        }
    }

    public TennnisPlayer(string name, Club club = null)
    {
        Name = name;
        Club = club;
    }

    public override string ToString()
        => $"{Name} ({Club?.Name})";
}

class Club
{
    public string Name { get; set; }
    
    List<TennnisPlayer> _members = new List<TennnisPlayer>();
    public IEnumerable<TennnisPlayer> Members { get => _members; }

    public Club(string name)
    {
        Name = name;
    }

    public void AddMember(TennnisPlayer player)
    {
        if (!_members.Contains(player))
        {
            _members.Add(player);
            player.Club = this;
        }
    }

    public void RemoveMember(TennnisPlayer player)
    {
        if (_members.Contains(player))
        {
            _members.Remove(player);
            player.Club = null;
        }
    }

    public override string ToString()
        => $"{Name}'s Members: " + string.Join(",", Members);
}

class Program
{
    static void Main()
    {
        var clubA = new Club("A");
        var clubB = new Club("B");
        Console.WriteLine(clubA);
        Console.WriteLine(clubB);

        var bjorn = new TennnisPlayer("Bjorn Borg");
        bjorn.Club = clubA;
        Console.WriteLine(bjorn);
        Console.WriteLine(clubA);
        Console.WriteLine(clubB);

        bjorn.Club = clubB;
        Console.WriteLine(bjorn);
        Console.WriteLine(clubA);
        Console.WriteLine(clubB);

        bjorn.Club = null;
        Console.WriteLine(bjorn);
        Console.WriteLine(clubA);
        Console.WriteLine(clubB);

        var john = new TennnisPlayer("John McEnroe", clubA);
        Console.WriteLine(john);
        Console.WriteLine(clubA);
        Console.WriteLine(clubB);

        john.Club = clubB;
        Console.WriteLine(john);
        Console.WriteLine(clubA);
        Console.WriteLine(clubB);

        john.Club = null;
        Console.WriteLine(john);
        Console.WriteLine(clubA);
        Console.WriteLine(clubB);
    }
}

 

[NoUserHere]

I think it's sad that you went through all of that explanation because i disagreed with you off the board regarding how this was or should be done. The points you've made above; I disagree with and I don't really care to discuss it with you any further. As I spent enough time discussing this off of the board.

Each person has their own ways of writing code, and some of us like to follow a certain set of principles over others, and that's also fine. So rather than going nitpicking and trying to make a big ordeal about it, just accept that you have your reasons for writing your application as you also demonstrated, and know that I have my way of doing it. The discussion you had off the board discussed these points already.

To say that the relationships are broken between each of the tuples is ridiculous and also wrong, nor is the relationship of tennisplayers and clubs not represented by association. Because they clearly are. They are stored in a list as you've cleverly demonstrated. Each entry in the list contains a single Tuple<tennisplayer, club> and prevention of a player being a member or any club twice can be done using simple Linq syntax just as I've done when removing members from a club in the code I presented. It requires little effort to prevent a user from being allowed into a list with a little checking.

And I stand by my reasoning, that there is no reason each of the objects should hold any data -- be it a list of tennisplayer, or a list of clubs. They are not required to know anything about each other, and if you really wanted them too, you should use inheritance. What you currently suggest also defeats the purpose of giving a class a single purpose. Lastly, If you look at my example carefully, it leaves you with ONE object, not FOUR. WithMembersOfClubs is the only object you need to pass around your program for the duration of its lifetime or for the lifetime of how long you need to work with the List defined therein. I envisioned that whoever uses the example I provided would have the common sense to create the instantiated objects in a function where they would pass in the club or tennis player details for a function to return either object.

Either way, that's as much energy as I will be given to this topic as I've already argued these points off the board. Happy programming, each to their own, after all they are both object orientated simply designed differently.

 

[NoUserHere]

@Matthieu - Skip to the last code block to see the main source block of code and to save reading how it works and what I changed.

Here's an improvement of the previous version I did. I switched out the tuples (not because there was anything wrong with using them) - there wasn't, but because I didn't use a list in the following example. I've switched types. The only reason i previously used a list was because the OP wanted to use a list, and tuples where fitting for that example. I also could have used a SortedList which allows you to add a KeyValuePair. Anyway, I switched that list for a ConcurrentDictionary of <TennisPlayer, Club>. ConcurrentDictionary's allow you to interact with them from any thread, and they are thread-safe dictionaries. I mainly chose this dictionary over the standard dictionary because it allows you to TryAdd/TryRemove which basically prevents duplicate entries being added ie. in the case of adding a user twice or to prevent them being apart of other clubs :

            if (membersOfClubs.AddTennisPlayerAsMember(tennisPlayer1, club))
                Debug.WriteLine(string.Join(" ", tennisPlayer1.PlayerName, "joined", club.ClubName));
            if (membersOfClubs.AddTennisPlayerAsMember(tennisPlayer1, club))
                Debug.WriteLine(string.Join(" ", tennisPlayer1.PlayerName, "joined", club.ClubName));

The additional convenience of TryAdd/TryRemove is that they return bools as they execute, and this makes checking for successful entries to the collection convenient. This also solves the problem of allowing a member to be part of more than one club, (whereas the previous example didn't prevent this, unless you placed a check to do so). As noted; that the TryAdd will only add a new tennis player if they are not already added, and calling this method returns a bool value.

I also tidied up the Linq expression that does the counting of the members in a given club. I done that by combining the where clause with count. Additionally I added a new function to return a list of member names of the tennis players from a given club. You can take a look at the below example which still removes the need for either tennis player class or the club class to know anything about each other. The reason I wrote it like this is because both classes only need to create a value or two according to the OP, ie. club name, club address etc. Previously I was using the constructor in place of these functions to create a new player or club and return them to the calling code. I've since changed that code to the following :

        public TennisPlayer InitializeNewPlayer(string playerName, string playerAddress, string playerPhoneNum) => new TennisPlayer
        {
            PlayerName = playerName,
            PlayerAddress = playerAddress,
            PlayerPhoneNumber = playerPhoneNum
        };
        public Club InitializeNewClub(string clubName, string clubAddress, string clubPhoneNum) => new Club
        {
            ClubName = clubName,
            ClubAddress = clubAddress,
            ClubPhoneNumber = clubPhoneNum
        };

Once you return the type, you can then try to add that new player to your collection. Another good thing about this collection is that you can also use ICollection and other interfaces for iterating collections (I didn't implement it). Another reason I prefer to use these classes as I've designed them, is because both club and tennisplayer have a single purpose, and that purpose is to provide a new reference of each new player(s)/club(s) details. This leaves us with a single class object which will hold all of the tennis players and the clubs they represent. That class is : WithMembersOfClubs. Here is the full example :

Main Code Block :

using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;

namespace WPFDataBinding
{
    public partial class Window1
    {
        public Window1()
        {
            InitializeComponent();
            WithMembersOfClubs membersOfClubs = new WithMembersOfClubs();

            /* Create your first club */
            Club club = membersOfClubs.InitializeNewClub("Bondi Club", "33 Bondibeach Park", "0123456789");
            /* Create your first player */
            TennisPlayer tennisPlayer1 = membersOfClubs.InitializeNewPlayer("Sheepings", "Sheep ville", "0234567890");

            /* If member does not exist, write that he joined the club */
            if (membersOfClubs.AddTennisPlayerAsMember(tennisPlayer1, club))
                Debug.WriteLine(string.Join(" ", tennisPlayer1.PlayerName, "joined", club.ClubName));

            /* Create your second player */
            TennisPlayer tennisPlayer2 = membersOfClubs.InitializeNewPlayer("Bob", "Bob ville", "3214567890");

            /* If member does not exist, write that he joined the club */
            if (membersOfClubs.AddTennisPlayerAsMember(tennisPlayer2, club))
                Debug.WriteLine(string.Join(" ", tennisPlayer2.PlayerName, "joined", club.ClubName));

            /* Lets display the number of members in a given club */
            Debug.WriteLine(string.Join(" ", club.ClubName, "has", membersOfClubs.NumberOfMembers_In(club.ClubName, 0), "Members"));

            /* Print the names of the players of a given club from the returned list */
            membersOfClubs.ListOfPlayerNames_By("Bondi Club").ForEach((string name) => Debug.WriteLine($"Member : {name}"));

            /* Remove the member from the given club if they exist */
            if (membersOfClubs.RemoveTennisPlayerAsMember(tennisPlayer1))
                Debug.WriteLine(string.Join(" ", tennisPlayer1.PlayerName, "left", club.ClubName));

            /* How many members remain after removing one? */
            Debug.WriteLine(string.Join(" ", club.ClubName, "has", membersOfClubs.NumberOfMembers_In(club.ClubName, 0), "Members"));
        }
    }
    public class Club
    {
        public string ClubName { get; set; }
        public string ClubAddress { get; set; }
        public string ClubPhoneNumber { get; set; }
    }
    public class TennisPlayer
    {
        public string PlayerName { get; set; }
        public string PlayerAddress { get; set; }
        public string PlayerPhoneNumber { get; set; }
    }
    /// <summary>
    /// Here you can use an additional class to work with the objects of your other classes; such as Club, and TenniPlayer.
    /// This class can add your tennis players to a ConcurrentDictionary<TennisPlayer, Club>. When this class is first instantiated,
    /// keep a copy of the instantiated object to pass around your application. New instances of this class will create a new instance of _PlayerClubPair.
    /// </summary>
    public class WithMembersOfClubs
    {
        /// <summary>
        /// This Concurrent Dictionary keeps track of your tenis players and the clubs they are in.
        /// </summary>
        private readonly ConcurrentDictionary<TennisPlayer, Club> _PlayerClubPair = new ConcurrentDictionary<TennisPlayer, Club>();
        /// <summary>
        /// The TennisPlayer is initialised with a new instance of a TennisPlayer object and its details are provided upon instantiation.
        /// </summary>
        /// <param name="playerName">The name of your tennis player</param>
        /// <param name="playerAddress">The address of your tennis player</param>
        /// <param name="playerPhoneNum">The phone number of your tennis player</param>
        /// <returns>Returns the TennisPlayer you created</returns>
        public TennisPlayer InitializeNewPlayer(string playerName, string playerAddress, string playerPhoneNum) => new TennisPlayer
        {
            PlayerName = playerName,
            PlayerAddress = playerAddress,
            PlayerPhoneNumber = playerPhoneNum
        };
        /// <summary>
        /// The Club class is initialised with a new instance of a club object and its details are provided upon instantiation.
        /// </summary>
        /// <param name="clubName">The name of the club is provided here</param>
        /// <param name="clubAddress">The club address is provided here</param>
        /// <param name="clubPhoneNum">The clubs phone number is provided here</param>
        /// <returns>Returns the Club you created</returns>
        public Club InitializeNewClub(string clubName, string clubAddress, string clubPhoneNum) => new Club
        {
            ClubName = clubName,
            ClubAddress = clubAddress,
            ClubPhoneNumber = clubPhoneNum
        };
        /// <summary>
        /// This method will add players to your _PlayerClubPair collection and store the values of your tennis player and club in a key value pair <TennisPlayer, Club>.
        /// </summary>
        /// <param name="tennisPlayer">This is where you pass in the tennis player object</param>
        /// <param name="clubName">This is where you pass in the club object</param>
        public bool AddTennisPlayerAsMember(TennisPlayer tennisPlayer, Club clubName) => _PlayerClubPair.TryAdd(tennisPlayer, clubName);
        /// <summary>
        /// This method will remove all references of a TennisPlayer object passed into it.
        /// </summary>
        /// <param name="tennisPlayer">This is where you pass in the tennis player you wish to remove</param>
        public bool RemoveTennisPlayerAsMember(TennisPlayer tennisPlayer) => _PlayerClubPair.TryRemove(tennisPlayer, out _);
        public List<string> ListOfPlayerNames_By(string clubName) => (_PlayerClubPair.Where(pair => pair.Value.ClubName == clubName).Select(pair => pair.Key.PlayerName)).ToList();
        /// <summary>
        /// This method allows you to check the number of members in a given club object.
        /// </summary>
        /// <param name="clubName">The name of the club you want to check the number of players in</param>
        /// <param name="number">The number variable will be used to count how many members are in a given club</param>
        /// <returns></returns>
        public int NumberOfMembers_In(string clubName, int number)
        {
            number += _PlayerClubPair.Values.Count(club => club.ClubName == clubName);
            return number;
        }
    }
}

Output :

Sheepings joined Bondi Club
Bob joined Bondi Club

Bondi Club has 2 Members

Member : Bob
Member : Sheepings

Sheepings left Bondi Club

Bondi Club has 1 Members

Finishing up...
The way that I have done this is mostly a matter of preference, and to stick to letting classes have a single purpose, where possible. I would always advocate creating classes that are based on contextual info like names and addresses to be left in that context so they retain their single use purpose. This does not make the above advice provided by the others invalid. Maybe they don't care to write code the way I do. And that's fine too. Each of us have our own principles and rules which we like to follow and break, and I'm sure we all have rules we also like to break when it's really convenient to do so. Sometimes we need to agree to disagree, because there is often no right or wrong way to do something when the functionality of something just works.

 

[Matthieu]

@Matthieu - Skip to the last code block to see the main source block of code and to save reading how it works and what I changed.

I want to thank you for the clear explanation and in the mean time I learned a lot of new things. It helped me a lot!

 

[NoUserHere]

You're very welcome. I'm glad you took away something from this topic.

Do you have any questions or are we good here?