Re: Coupling in OOP design?
On May 28, 8:18 am, andrewmcdonagh <andrewmcdon...@gmail.com> wrote:
On May 28, 1:02 pm, Lew <l...@nospam.lewscanon.com> wrote:
andrewmcdonagh wrote:
On May 28, 9:21 am, howa <howac...@gmail.com> wrote:
Consider the simplfied code below, do you think the design is good, or
bad?
class Person {
...
setMother(Person p) {
p.setSon(this);
this.mother = p;
}
...
}
any suggestions on improving the design?
Nothing wrong with this.
Its not 'coupling', as there is only one class 'Person' that
references (given the code above) other 'instances' of the same class.
Its a data (runtime) relationship.
OTOH there are many ways to model the relationahips implied in the example.
Not shown is whether the "son" attribute also handles "[step]daughter", or
"mother" encompasses "guardian/[step]father" as well.
Modeling every kind of relationship as an attribute in Person is valid but
potentially unwieldy. What if you decide to model aunts later? Cousins? At
least you have incest covered - if a girl marries her son and becomes her own
mother you can model it.
Other approaches include modeling the relationship as a composed object, or
member of a Collection of composed objects:
class Person
{
Collection<Relationship> relationships;
...
}
where Relationship has attributes of a Relation Enum and a target Person (or
tuple of Persons).
There are more ways to skin that cat as well.
--
Lew
Sure we 'can' do this...but the OP wanted to know if there was
anything intrinsically wrong with what they have, and given that its a
snippet of code there's little point over analysing it.
Andrew
PS: Besides, your enum Relationship is is a static state, which won't
take care of the dynamic happenings of real life. E.g. Mum becoming
Stepmum of her new husband's children, Step-sister becoming
Wife,etc. ;-)
Alternatively, you could say
Map<Person, List<Relation>> relationships;
At this point, we're talking about modeling the person and their
relationships as a Graph, and it might make sense to consider other
data model approaches.
Perhaps instead of the above map, we have:
final class Relation {
/* enum RelationType is defined somewhere else.
It defines the reciprocal as well:
RelationType.PARENT.reciprocal() == RelationType.CHILD;
Note: we leave gender out of the relationship type for many
reasons.
*/
private RelationType type;
private final Person person;
private Person relative;
public Relation(RelationType type, Person relative, Person person)
{
/* initialize here */
}
/* snip other constructors/accessors */
Relation reciprocal() {
return new Relation(type.reciprocal(), person, relative);
}
}
class Person {
final Collection<Relation> relationships =
new ArrayList<Relation>();
}
What we've ended up with here is a specific class of a Node (person)
and Edge (Relation).
So, my point is that there are many ways to model this. It all
depends on what you need from your model. A good engineer will likely
start with the easiest/simplest approach (such as the OP solution),
and refactor into the other models as they become appropriate for the
project.