javalaw-of-demeter

Does this change improve my design with regards to the Law Of Demeter?


Let's say I need write a Wrapper for a 3rd party class which I'm not able to change.

The interface of the class looks like this

class Rewriter {
    public List<Mapping> getMappings();
}

The wrapper looks like this

class RewriterSpec {
    private final Rewriter rewriter;

    public RewriterSpec(Rewriter rewriter) {
        this.rewriter = rewriter;
    }

    public addMapping(Mapping m) {
        rewriter.getMappings().add(m);
    }
}

So from my understanding the RewriterSpec violates the Law of Demeter because it requires structural knowledge about the Rewriter.

Now, the question is, would it be better from a design / testing point of view to just pass in the list of mappings?

class Rewriter {
    public List<Mapping> getMappings();
}

The wrapper looks like this

class RewriterSpec {
    private final Rewriter rewriter;

    public RewriterSpec(List<Mapping> mappings) {
        this.mappings = mappings;
    }

    public addMapping(Mapping m) {
        mappings.add(m);
    }
}

Is it okay to just pass the List by reference?


Solution

  • Wikipedia states the following:

    https://en.wikipedia.org/wiki/Law_of_Demeter

    More formally, the Law of Demeter for functions requires that a method m of an object O may only invoke the methods of the following kinds of objects:[2]

    • O itself
    • m's parameters
    • Any objects created/instantiated within m
    • O's direct component objects
    • A global variable, accessible by O, in the scope of m

    By following these principals, you could define a method in the Rewriter interface to directly add Mapping objects to its list to satisfy the principle (A global variable, accessible by O, in the scope of m).

    class RewriterSpec {
        private final Rewriter rewriter;
    
        public RewriterSpec(Rewriter rewriter) {
            this.rewriter = rewriter;
        }
    
        public addMapping(Mapping m) {
            rewriter.addMapping(m);
        }
    }
    

    Considering Rewriter can't be modified, you can opt for the following:

    class RewriterSpec {
        private final Rewriter rewriter;
        private final List<Mapping> mappings;
    
        public RewriterSpec(Rewriter rewriter) {
            this.rewriter = rewriter;
            this.mappings = rewriter.getMappings();
        }
    
        public addMapping(Mapping m) {
            mappings.addMapping(m);
        }
    }