DDD Aggregate for seat booking in cinema - is my model correct?

Question

I created a DDD aggregate for booking seats in cinema. Is it corect or it should be model in other way?

Business rules:

1.Seat can be booked at least 1h before screening

2.Screening can has only 1 active booking for seat

3.Booking can be cancelled at least 24h before screening

public class Screening {

private Long id;

private LocalDateTime date;

private List<Seat> seats;

private List<Booking> bookings;

public Screening(Long id, LocalDateTime date, List<Seat> seats) {
    this.id = id;
    this.date = date;
    this.seats = seats;
    this.bookings = new ArrayList<>();
}

public Booking addBooking(
        Booking booking,
        LocalDateTime currentDate,
        int rowNumber,
        int seatNumber
) {
    if (this.timeToScreeningInHours(currentDate) < 1) {
        throw new BookingTooLateException();
    }
    var foundSeat = findSeat(rowNumber, seatNumber);
    if (this.hasActiveBooking(foundSeat)) {
        throw new BookingAlreadyExists();
    }
    booking.markAsActive(this, foundSeat);
    this.bookings.add(booking);
    return booking;
}

public void cancelBooking(LocalDateTime currentDate, Long bookingId, Long userId) {
    if (this.timeToScreeningInHours(currentDate) < 24) {
        throw new BookingCancelTooLateException();
    }
    this
            .findBooking(bookingId, userId)
            .markAsCancelled();
}

private int timeToScreeningInHours(LocalDateTime currentDate) {
    return (int) Duration
            .between(currentDate, date)
            .abs()
            .toHours();
}

private Seat findSeat(int rowNumber, int seatNumber) {
    return this
            .seats
            .stream()
            .filter(s -> s.placedOn(rowNumber, seatNumber))
            .findFirst()
            .orElseThrow(() -> new EntityNotFoundException("Seat"));
}

private boolean hasActiveBooking(Seat seat) {
    return this
            .bookings
            .stream()
            .anyMatch(booking -> booking.isActiveOn(seat));
}

private Booking findBooking(Long bookingId, Long userId) {
    return this
            .bookings
            .stream()
            .filter(booking -> booking.hasId(bookingId) && booking.hasUserId(userId))
            .findFirst()
            .orElseThrow(() -> new EntityNotFoundException("Booking"));
   }
}

Answer 1

There is some review context missing: the Seat class.

---

use appropriate data structure

private List<Seat> seats;
...
private Seat findSeat(int rowNumber, int seatNumber) {
    return this
            .seats
            .stream()
            .filter(s -> s.placedOn(rowNumber, seatNumber)) ...

This apparently has O(N) cost, linear in number of seats.

Recommend you implement the collection of Seats with a HashSet, for O(1) constant lookup speed.

Similarly for Bookings.

---

pass in an appropriate type

private Seat findSeat(int rowNumber, int seatNumber) {
            ...
            .filter(s -> s.placedOn(rowNumber, seatNumber))

It seems odd for caller to supply a pair of integers, rather than passing in a Seat object.

The findBooking input parameters look fine. But testing ... && booking.hasUserId(userId) seems odd. It's fine, it's correct. Rather than inviting caller to guess the correct userId, I was expecting ... && booking.getUserId() == userId