nus-cs2103-AY2425S1 / forum

12 stars 0 forks source link

Law of Demeter query #546

Open seah-minlong opened 2 days ago

seah-minlong commented 2 days ago

If I understood Law Of Demeter correctly, the following violates it

public void foo(Person p1, Person p2) {
  return p1.getName().compareName(p2.getName());
}

However, would doing the following make it not violate LoD then?

public void foo(Person p1, Person p2) {
  return goo(p1.getName(), p2.getName());
}

public void goo(Name n1, Name n2) {
  return n1.compareName(n2);
}

If so, then what would the difference be really, since I am still kind of navigating the internal structures of other objects.

image

KrashKart commented 2 days ago

I guess the difference here is that in the first code block, the Name objects (which are a friend of a friend) are not declared as an input to the method foo, meaning they are "strangers". In the second code block, technically the Name objects are direct parameters to goo, thus are not considered strangers.

My take would be that despite this technicality, the idea of LoD is still violated because the internal structure of Person is still traversed.

damithc commented 2 days ago

@seah-minlong Good question. Let's wait for inputs from others.

kimxw commented 2 days ago

I agree with @KrashKart that both violate LoD Although the notes say:

More concretely, a method m of an object O should invoke only the methods of the following kinds of objects:

  • The object O itself
  • Objects passed as parameters of m
  • Objects created/instantiated in m (directly or indirectly)
  • Objects from the direct association of O

I believe it implies that

More concretely, a method m of an object O should *directly or indirectly* (by calling another method of the object O) invoke only the methods of the following kinds of objects: ...

I could be wrong though

seah-minlong commented 2 days ago

I do agree even if this doesn't technically violate LoD, it would violate the idea of LoD.

Would making compareName() a method in Person be better then? My main concern with regards to doing this is if it will be violating Single Responsibility Principle for Person or should this question be left to another thread.

zaidansani commented 2 days ago

I assume you're trying to sort your Person through the name, hence the comparing of names.

I know this isn't what you're looking for specifically, but would it not be easier, in that sense, to implement Comparable and thus use the compareTo method for whatever purpose you need?

--

Also, would SRP be broken for Person in this scenario? Would the addition of the method be considered an additional responsibility - it's not necessarily implementing the compareTo itself, but instead grabbing the result of the Name comparison. I would argue it would be part of the responsibility of the Person class - which is to model the data and behaviour of a person.

seah-minlong commented 1 day ago

Thanks for the suggestion! Implementing Comparable with the compareTo() method for name comparison is definitely a straightforward approach. However, in my case, I need more flexibility because I want to compare Person objects based on different attributes, such as name, or other fields, depending on the context.

I see your point about SRP. In this case, I’m essentially using Person as a means to access its properties for comparison, without introducing new responsibilities. Would that be what you mean @zaidansani?