OOP at the boundary

Question

I'm trying to implement a simple service in the true OOP way. This means for me that the domain objects has no technical or non-business-related methods (in practice no getters/setters).

Here are the domain interfaces to implement:

public interface Person {
    void register();
    void changeName(String name);
}

public interface Persons {
    Person register(String name);
    Person findByName(String name);
}

Solution #1

@AllArgsConstructor
public class RegisteredPerson implements Person {
    private Long id;
    private String name;

    private final PersonRegistry registry;

    public RegisteredPerson(String name, PersonRegistry registry) { 
        this.name = name; 
        this.registry = registry;
    }

    public void register() {
        if (id != null) {
            throw new PersonAlreadyRegisteredException(name);
        }
        id = registry.register(name);
    }

    public void changeName(String name) {
        this.name = name;
        if (id != null) {
            registry.changeName(id, name);
        }
    }
}

@RequiredArgsConstructor
public class RegisteredPersons implements Persons {    
    private final PersonRegistry registry;

    public Person register(String name) {
        if (registry.byName(name).isPresent()) {
            throw new PersonAlreadyRegisteredException(name);
        }
        Person person = new RegisteredPerson(name, registry);
        person.register();
        return person;
    }

    public Person findByName(String name) {
        return registry.byName(name)
            .orElseThrow(() -> new PersonNotFoundException(name));
    }
}

I introduced a new non-public interface for a repository:

interface PersonRegistry {
    long register(String name);
    Optional<Person> byName(String name);
    void changeName(long id, String name);
}

class InMemoryPersonRegistry implements PersonRegistry {    
    private final Map<Long, PersonEntry> personEntries = new HashMap<>();
    private final AtomicLong idSequence = new AtomicLong();

    public long register(String name) {
        long id = idSequence.incrementAndGet();
        personEntries.put(id, new PersonEntry(id, name));
        return id;
    }

    public Optional<Person> byName(String name) {
        return personEntries.values().stream()
                .filter(entry -> name.equals(entry.name))
                .findAny()
                .map(entry -> new RegisteredPerson(
                    entry.id, entry.username, this
                ));
    }

    public void changeName(long id, String name) {
        if (personEntries.containsKey(id)) {
            personEntries.get(id).name = name;
        }
    }

    @AllArgsConstructor
    @EqualsAndHashCode(of = "id")
    private class PersonEntry {
        public long id;
        public String name;
    }
}

What I don't like about this solution it the coupling between InMemoryPersonRegistry and RegisteredPerson. For example, In a situation when the RegisteredPerson has more dependencies, all of them must be known to the InMemoryPersonRegistry. Possible solution for this would be to introduce a factory and make the repository (registry) dependent only on that factory. But the coupling still doesn't feel alright.

The repository is hard to test as well, because the returned object has no getters to prove correctness of the persistence.

Solution #2

In this version I implemented the registry as a dumb DAO with no business meaning, only as a tool to persistent entries. This breaks the coupling and makes the repository implementation independent of the other world.

interface PersonEntries {
    long save(PersonEntry entry);
    Optional<PersonEntry> byName(String name);
    void updateName(long id, String name);

    @RequiredArgsConstructor
    class PersonEntry {
        public final Long id;
        public final String name;
    }
}

On the other hand it's much more boiler-plate code on the client side:

public class RegisteredPerson implements Person {
    private final PersonEntries entries;
    /* ... */
    public void register() {
        if (id != null) {
            throw new PersonAlreadyRegisteredException(name);
        }
        id = entries.save(new PersonEntries.PersonEntry(null, name));
    }

    public void changeName(String name) {
        this.name = name;
        if (id != null) {
            entries.updateName(id, name);
        }
    }   
}

public class RegisteredPersons implements Persons {    
    private final PersonEntries entries;
    /* ... */

    public Person findByName(String name) {
        return entries.byName(name)
            .map(entry -> new RegisteredPerson(entry.id, entry.name, entries))
            .orElseThrow(() -> new PersonNotFoundException(name));
    }
}

When a Person structure changes, a lot of places in code must be modified.

Question is whether it makes sense to have the method updateName in the PersonEntries or would it be better to update everyting via save method.

Another point is the return value of the save, would it be better to update the entry and have void instead:

// RegisteredPerson.register():
PersonEntries.PersonEntry entry = new PersonEntries.PersonEntry(null, name);
entries.save(entry);
this.id = entry.id;

Any thoughts and ideas are appreciated! Thank you!

Answer 1

I find it a bit conflicting that the register() method has been put in the Person interface. Should the Person be responsible for registering itself at all? It also creates a bit of a confusion in the RegisteredPerson class, whose name implies that it is registered when it is in fact unregistered until it's register() method is called successfully.

In true OO fashion, if RegisteredPerson is a public class, it should not be possible to be instantiate it unless it is actually registered in the registry. In my opinion, if you want to differentiate UnregisteredPerson and RegisteredPerson in the class hierarchy, they should have their own interfaces and the implementation should be made in a way that the instantiation (and existence) of concrete RegisteredPerson objects is completely hidden inside the registry.

Something along these lines:

interface PersonRegistry {
    RegisteredPerson findByName(String name);
    RegisteredPerson register(UnregisteredPerson person);
}