Übung »Code Review of Pre-Task for DDD Master Course 2025«

The Coding Pre-Task in the The Domain-Driven Design Master course is supposed to make you familiar with the Spring Modulith Framework. It is not graded. You only need to pass all tests, whatever the code quality is like. This is a deliberate decision. The goal of this assignment is just to let you work with the framework, without the pressure to do a “high quality job” (that will come later in the DDD course, when you implement the case study). However, it would certainly make sense for you to get feedback on your code quality - and this is what this page is for.

Dauer

Ca. 60 min

Content

• What is this about?
• Aggregate Root Implementation with Issues (1)
• Aggregate Root with Issues (2)
• Something Wrong with Aggregate Root Repository
• Something Wrong with the Service
• What could be improved in this module structure?
• What is wrong with this Aggregate Root and Service Class?

What is this about?

I have identified a couple of patterns and bad smells in your code, and denoted in the solution (in an anonymous fashion, only by listing the UUID of your personal repo) in which cases this bad smell or pattern could be found. Your task is to identify what is wrong with the code snippets below.

Aggregate Root Implementation with Issues (1)

Please have a look at an Aggregate Root and part of the service class where instances of that Aggregate Root are created. There are several issues with this code. What is wrong with it?

Aggregate Root

package thkoeln.archilab.ddd.associate;

import jakarta.persistence.*;

import java.time.LocalDate;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;

@Entity
@Table(name = "associates")
public class Associate {

    @Id
    private String id;

    private String name;

    @ElementCollection
    @CollectionTable(name = "business_trips", joinColumns = @JoinColumn(name = "associate_id"))
    private List<BusinessTrip> trips = new ArrayList<>();

    public Associate() { }

    public Associate(String id, String name) {
        this.id = Objects.requireNonNull(id);
        this.name = Objects.requireNonNull(name);
    }
    
    //...
 }

Application Service

package thkoeln.archilab.ddd.associate;
// ...
@Service
@Transactional
public class AssociateService implements AssociateAPI {

    private final AssociateRepository repo;
    private final ApplicationEventPublisher publisher;

    public AssociateService( AssociateRepository repo, ApplicationEventPublisher publisher ) {
        this.repo = repo;
        this.publisher = publisher;
    }

    @Override
    public Identifier add( String name ) {
        if ( name == null || name.trim().isEmpty() ) throw new TravelException( "name null/empty" );
        AssociateId id = new AssociateId();
        Associate a = new Associate( id.toString(), name.trim() );
        repo.save( a );
        return id;
    }

    // ...
}

Aggregate Root with Issues (2)

Similar as above - what is wrong with this Aggregate Root and service class?

Aggregate Root

package thkoeln.archilab.ddd.companycar;

import org.jmolecules.ddd.types.Identifier;
import thkoeln.archilab.ddd.api.TravelException;

import java.time.LocalDate;
import java.util.ArrayList;
import java.util.List;
import java.util.UUID;

/**
 * Aggregate Root representing a physical company car.
 * Manages its own schedule of reservations and availability status.
 */
public class CompanyCar {
    private final Identifier id;
    private final LocalDate availableFrom;
    private final List<CarReservation> reservations = new ArrayList<>();
    
    //...
 }

Application Service

package thkoeln.archilab.ddd.companycar;
//...
@Service
public class CompanyCarService implements CompanyCarAPI {
    private final ApplicationEventPublisher eventPublisher;
    // Simulating a database
    private final Map<Identifier, CompanyCar> repository = new HashMap<>();
    // ...
}

Something Wrong with Aggregate Root Repository

What is wrong with this Aggregate Root repository?

package thkoeln.archilab.ddd.rentalcar;

import org.springframework.data.jpa.repository.JpaRepository;

public interface RentalCarRepository extends JpaRepository<RentalCar, RentalCarId> {
}

Something Wrong with the Service

What is wrong with this service class?

package thkoeln.archilab.ddd.associate;

import org.springframework.context.ApplicationEventPublisher;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import org.jmolecules.ddd.types.Identifier;
import thkoeln.archilab.ddd.api.AssociateAPI;
import thkoeln.archilab.ddd.api.TravelException;
import java.time.LocalDate;

@Service
@Transactional
public class AssociateService implements AssociateAPI {
    // ...
}

What could be improved in this module structure?

This module structure is not wrong, but it could be improved. What would you suggest?

What is wrong with this Aggregate Root and Service Class?

The code below is not wrong, but shows a common anti-pattern. What is the problem here?

Aggregate Root

package thkoeln.archilab.ddd.employee;
//...
@Getter
public class Employee extends AbstractAggregateRoot<Employee>
        implements AggregateRoot<Employee, EmployeeId> {

    private final EmployeeId id;
    private final String name;

    public Employee(String name) {
        this.id = new EmployeeId(UUID.randomUUID());
        this.name = name;
    }
}

Application Service

package thkoeln.archilab.ddd.employee;
//...

@Transactional
@Service
@RequiredArgsConstructor
public class EmployeeService implements EmployeeAPI {
    //...
    public void planBusinessTrip(Identifier id, LocalDate from, LocalDate until, boolean companyCarNeeded) {
        if (id == null || from == null || until == null || until.isBefore(from)) {
            throw new TravelException("invalid parameters");
        }
        if (!(id instanceof EmployeeId employeeId)) {
            throw new TravelException("id must be an EmployeeId");
        }
        if (!employeeRepo.findById(employeeId).isPresent()) {
            throw new TravelException("Employee not found.");
        }
        List<BusinessTrip> trips = businessTripRepo.findAll();
        for (BusinessTrip trip : trips) {
            if (trip.getEmployeeId().pointsTo(employeeId)) {
                LocalDate existingFrom = trip.getFromDate();
                LocalDate existingUntil = trip.getUntilDate();
                if (!until.isBefore(existingFrom) && !from.isAfter(existingUntil)) {
                    throw new TravelException(
                            "There is an overlap with an already planned business trip for this employee.");
                }
            }
        }
        businessTripRepo.save(new BusinessTrip(employeeId, from, until, companyCarNeeded));
        if (companyCarNeeded) {
            events.publishEvent(new CompanyCarReservedEvent(employeeId, from, until));
        }
    }
    //...
}