Skip to main content
22 events
when toggle format what by license comment
Nov 22, 2017 at 13:37 comment added w0051977 I have added an update to my OP. Could you review it before I mark your answer?
Nov 22, 2017 at 11:38 comment added Neil AddSport is preferable. If you feel you need AddSports method, then have both, with AddSports repeatedly calling AddSport. Or again, if Sports is a property, someone could simply do person.Sports.Add(sport), eliminating the need to have an AddSport method at all.
Nov 22, 2017 at 11:29 comment added w0051977 I have added a comment in my OP above AddSports and I have added some properties to: Person. This may help explain my misunderstanding. I don't know from your last comment whether you are saying have a method called: AddSport or AddSports. I realise this is subjective, however it would be good to get an opinion based on the information I have provided. Thanks again.
Nov 22, 2017 at 11:11 comment added Neil @w0051977 If you can add one sport, you can add many sports without providing a method that does this for you. Also the existence of an AddSports method (as opposed to an AddSport method) makes me think handling a list of sports is somehow different than handling a sport one at a time. That isn't your case I presume, so it would be misleading to someone reading your code. However again, you're free to do as you choose.
Nov 22, 2017 at 11:01 comment added w0051977 @Neil, thanks again - I am nearly there. I have shown what AddSports does in the question. I could change it to AddSport, but then the service layer would have to loop through all the Sports and pass each sport to Person.AddSports(). I believe the code in my question is "better". However, as I say - I cannot find a single example where a list is passed to a domain layer method rather than an object.
Nov 22, 2017 at 10:56 comment added Neil @w0051977 Because a method accepting one Sport instance is adaptable whereas accepting a List of Sport may involve transforming into a List if only to call the method. If anything you have an AddSport method and a SetSports. Or better still, if no logic is to be performed, just make Sports a property of Person (not that you couldn't have logic in the setter, but it isn't best practice). It's not necessarily worse, but it's generally more accepted to have an AddSport and maybe a AddSports but not vice versa.
Nov 22, 2017 at 10:17 comment added w0051977 Can I ask one more think before I accept? Is it bad practice to pass a list do a domain method i.e. in my case I am passing a list of Sports to Person.AddSport? I do this without a second thought. However, I have reviewed a few code bases on GitHub e.g. NHibernate and I cannot find a single example where this is done. Every single example I find passed an object to a method rather than a list of objects.
Nov 22, 2017 at 4:16 comment added CodingYoshi A composition relationship would imply that Sports make up Person is not quite correct. It is not equivalent to parts make a whole. It's more about who owns whom. For example consider a person to cars--cars do not make a person and neither is a person made of cars but nonetheless composition can exist.
Nov 21, 2017 at 15:36 comment added Neil @w0051977 Yes, that sounds about right.
Nov 21, 2017 at 15:30 comment added w0051977 Thank you. I think what you are saying is that this is an Association relationship rather than a Composition relationship. Have I understood that correctly? I would agree with that.
Nov 21, 2017 at 15:25 comment added Neil @w0051977 Your PersonSport database object is PersonCanPlaySport that matches the two tables in many-to-many relationship. In your code, if you don't need to take advantage of the many-to-one Person to Sport relationship, you can effectively ignore it. However I would still avoid calling it a composition since it isn't a composition relationship on the database. That said, if you think your code works very well, you're free to do as you choose, ignoring my advice.
Nov 21, 2017 at 15:05 comment added w0051977 Thanks. However, everywhere I read tells me not to create a junction class unless it is strictly necessary. There is no such thing as a PersonSport in the Ubiqituous language after all - it is a database concept. The code in my OP works very well, however I am consfused whether the relationship between person and sport (many to none) is an Association or composition - it appears to be both.
Nov 21, 2017 at 14:48 comment added Neil @w0051977 Updated the answer.
Nov 21, 2017 at 14:48 history edited Neil CC BY-SA 3.0
added 632 characters in body
Nov 21, 2017 at 14:27 comment added w0051977 I notice that you have not answered one of the questions in my last paragraph. Is there a Composition relationship and an Association relationship between the two classes? Is this a code smell? I ask because Person.AddSports has a list of Sports passed to it populated externally to the class: Person. Thanks.
Nov 21, 2017 at 11:25 vote accept w0051977
Nov 21, 2017 at 15:23
Nov 21, 2017 at 11:23 comment added Neil @w0051977 Yes, essentially. If they need to know about each other, a third class should be added to mediate. Otherwise, your approach is fine.
Nov 21, 2017 at 11:12 comment added w0051977 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?
Nov 21, 2017 at 10:59 comment added Neil @w0051977 Your way would work too, but you're still tightly coupling Person and Sport this way. If you tell me Sport doesn't have a list of Persons, then you could eliminate the necessity for Sport to know about Person entirely, but it means your method CanPlay needs to be added to Person with a Sport instance passed to it. So long as Sport doesn't need to know about Person, you'll have no conflict. Otherwise, I would recommend my approach.
Nov 21, 2017 at 9:53 comment added w0051977 For the reference to an intermediary class +1. It is one way of approaching it.
Nov 21, 2017 at 9:52 comment added w0051977 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?
Nov 21, 2017 at 9:38 history answered Neil CC BY-SA 3.0