Say I have a many to many relationship in the database like this:
CREATE TABLE Person (ID int, name varchar(100), dateofbirth datetime, Gender char(1), primary key (ID))
CREATE TABLE Sport (ID int, description varchar(30), primary key (ID))
CREATE TABLE PersonCanPlaySport (Person ID int references Person(ID), SportID int references Sport(ID), PRIMARY KEY (SportID, PersonID))
The tables show what sports a person can participate in. Now see the class below:
public class Person
{
public List<Sport> Sports;
public int FitnessLevel;
public datetime DateOfBirth;
public string Gender; //M or F
public Person()
{
Sports = new List<Sport>();
}
//allSports contains all the sports from the database i.e. allSports contains about 100 sports. AddSports determines what sports (out of the 100) that the user can participate in based on: Gender, age, Fitness Level etc
public void AddSports(List<Sports> allSports)
{
Sports.Clear();
((List<Sport>)Sports).AddRange(allSports
.Where(sport => sport.CanPlay(this)));
}
}
There appears to be an Association relationship and a Composition relationship between Person and Sports. Here is why:
1) Person creates an instance of a list of sports in the constructor so controls the lifecycle of Sports 2) Person uses a List of sports in AddSports, which I believe makes it Association
Is this a code smell? What kind of relationship exists between the two objects? How would this be represented on an Object Diagram?
Update
Following on from the helpful answer and comments from @Neil; I believe he is suggesting doing this:
//Application Service Layer
List<Sport> sports = Repository.GetAllOffers();
foreach sport As Sport in sports
{
person.AddSports(sport);
}
//Domain class
public class Person
{
public List<Sport> Sports;
public int FitnessLevel;
public datetime DateOfBirth;
public string Gender; //M or F
public Person()
{
Sports = new List<Sport>();
}
public void AddSport(Sport sport)
{
if(sport.CanPlay(this))
{
Sports.Add(sport);
}
}
}
I am not sure if I like the idea that the Application Service layer has to loop through the Sports.
1 Answer 1
The database relationship between Person
and Sport
as I understand it is many-to-many, with a intermediate table PersonCanPlaySport
that connects the two. This means a Person
can be associated with many Sports
and a Sport
can be associated with many Persons
.
Now that we understand the database relationship, the relationship used in the code should be no different. Potentially a Person
instance would have a list of Sport
and a Sport
instance would have a list of Person
.
I think therefore it would be incorrect to put the logic of whether or not a Person
should have a specific Sport
directly inside Person
, or vice versa. Consider the use of a PersonSportDao
object which given a Person
, can return all Sports
and given a Sport
can return all Persons
. The caller to AddSports
would make the call to PersonSportDao
, or better still, Person
instance itself loads its Sports
when asked for it, calling PersonSportDao
with this
.
If you wish to check details regarding age or gender, you could create another class PersonSportManager
. In other words, Person
isn't owner of Sport
and so it should see none of the logic just as Sport
isn't owner of Person
and so it shouldn't see any of the logic of Person
.
Had Person
to Sport
been a one-to-many relationship, I would have argued otherwise, but I think the code should reflect the symmetry of the relationship on the database.
Edit: Answering your question:
Is this a code smell? What kind of relationship exists between the two objects? How would this be represented on an Object Diagram?
A composition relationship would imply that Sports
make up Person
which is like a one-to-many relationship between Person
and Sport
. In your case that isn't true, being many-to-many.
The relationship is too complicated to simply draw a line between the two making it necessary to create a middle class, and then associating Person
and Sport
both to that middle class (such as PersonSportManager
).
Hope that answers your question!
-
Adding an intermediary class is one way of resolving the impedance mismatch. However, instead I have decided to: 1) Add a list of Sports to Person and 2) Not add a List of persons in Sport. This makes the object relationship 0:M. I have edited the question by adding code to the AddSports method. Could you tell me what you think now?w0051977– w005197711/21/2017 09:52:41Commented Nov 21, 2017 at 9:52
-
For the reference to an intermediary class +1. It is one way of approaching it.w0051977– w005197711/21/2017 09:53:33Commented Nov 21, 2017 at 9:53
-
@w0051977 Your way would work too, but you're still tightly coupling
Person
andSport
this way. If you tell meSport
doesn't have a list ofPersons
, then you could eliminate the necessity forSport
to know aboutPerson
entirely, but it means your methodCanPlay
needs to be added toPerson
with aSport
instance passed to it. So long asSport
doesn't need to know aboutPerson
, you'll have no conflict. Otherwise, I would recommend my approach.Neil– Neil11/21/2017 10:59:35Commented Nov 21, 2017 at 10:59 -
Thank you. So what you are saying is - use my approach is Sport does not need to know about person and use your approach if it does?w0051977– w005197711/21/2017 11:12:06Commented Nov 21, 2017 at 11:12
-
@w0051977 Yes, essentially. If they need to know about each other, a third class should be added to mediate. Otherwise, your approach is fine.Neil– Neil11/21/2017 11:23:30Commented Nov 21, 2017 at 11:23
Person
controls the lifecycle of theSport
elements in that collection. (It does, however, control/own the lifecycle of the collection object itself — but that is a very low level relationship: merely an artifact of (collections in) objects.)AddSports
with sports that the person could not play or take part in? But about your question of composition and association: even if you call this composition, the list of sports I passed in could have existed beforePerson
object came into existence and may even exist afterPerson
is garbage collected. Now you may say "ok so then its association"--maybe but maybe not. Depends on what you do with it.