Law of Demeter confusion

Question

I'm hoping someone can help explain the law of demeter to me. If I have a class which I'm assuming is an aggregate root and within that have a collection of child classes is it illegal to update the properties of those child classes by accessing them through the aggregate root?

e.g.

public class Company
{
    // company has a number of employees
    public List<Employee> Employees {get; set;}
}

public class Employee
{
    // each employee has a lastname
    public int Id {get; set;}
    public string LastName {get; set;}
    // other properties of employee
}

lets say I have a client that is accessing the Company class firstly would it be violating the law of demeter with something like.

Employee e = aCompany.Employees.Where(e => e.Id == 1).Single();
e.LastName = "MarriedName";

Or should this always be delegated to Company

public class Company
{
    public UpdateEmployeeLastName(int employeeId, string newName)
    {
        Employee e = Employees.Where(e => e.Id == employeeId).Single();
        e.LastName = newName;
    }
}

in the client

aCompany.UpdateEmployeeLastName(1, "Marriedname");

The second one seems better but is there anything wrong with the client having to know the id of the Employee it wants to update?

This seems like it could start to get complicated where you have a number of nested aggregates.

Thanks

Gao

Answer 1

Your second option is what the Law of Demeter aims for.

Since the Law of Demeter basically states "only talk to what you know about".. whatever the "client" is in the first scenario doesn't actually know about employees at all. It knows about a Company.. but not about the intricacies of the Company internals.

Delegating to the Company gives you the flexibility to change how an employee is updated without having to change each specific instance of this functionality from the client. If one day you decide that only Active employees can have their names changed, then you would have to update every instance of option one to this:

Employee e = aCompany.Employees.Where(e => e.Id == 1 && e.IsActive).Single();
//                                                        ^^^^ active flag
e.LastName = "MarriedName";

Wrapping it up in Company makes this much nicer to deal with in future (regardless of attempting to follow the Law of Demeter or not).

The second one seems better but is there anything wrong with the client having to know the id of the Employee it wants to update?

Both of your examples know the ID of the Employee.. so I'm not sure what you mean by this. It is very common for consuming code to be aware of the ID when passing information through an Aggregate.

Herzegovina

Thank you. I think I just needed someone to validate my thinking :) – Gao

Answer 2

The post Visualization Mnemonics for Software Principles by Erik Dietrich provides a very effective trick to understand (and never forget anymore) what the Law of Demeter is.

An example of code violating the Law of Demeter

class Plane {
    constructor(crew) {
        this.crew = crew;
    }
    
    getPilotsName() {
        this.crew.pilot.getName();
    }
}

class Crew {
    constructor(pilot) {
        this.pilot = pilot;
    }
}

class Pilot {
    getName() {
        // ...
    }
}

It's bad, because it creates tight coupling between objects - they are dependent on internal structure of other objects.

Fixed code:

class Plane {
    getPilotsName() {
        this.crew.getPilotsName();
    }
}

class Crew {
    constructor(pilot) {
        this.pilot = pilot;
    }

    getPilotsName() {
        return this.pilot.getName();
    }
}

Dixson