Agregat do rezerwacji miejsc w kinie (DDD)

Czy tak powinien wyglądać agregat z DDD do rezerwowania miejsc w kinie, czy można by to jakoś lepiej zamodelować?

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"));
    }
}

Troche to wygląda jakbyś sam nie wiedział co to ma robić.

Żeby zacząć od DDD, to musiałbyś wiedzieć jakie funkcje ma spełniać ta aplikacja - co ona ma robić?

Ma znaleźć wszystkie dostępne miejsca na jakiś film we wszystkich kinach, pokazać je userowi żeby ten mógł wybrać jedno, i potem kupić jakieś miejsce?

Bo jeśli tak, to raczej trochę to na około zrobiłeś.

Riddle napisał(a):

Troche to wygląda jakbyś sam nie wiedział co to ma robić.

Żeby zacząć od DDD, to musiałbyś wiedzieć jakie funkcje ma spełniać ta aplikacja - co ona ma robić?

Ma znaleźć wszystkie dostępne miejsca na jakiś film we wszystkich kinach, pokazać je userowi żeby ten mógł wybrać jedno, i potem kupić jakieś miejsce?

Bo jeśli tak, to raczej trochę to na około zrobiłeś.

Nie, to apka do jednego kina

Nofenak napisał(a):

Riddle napisał(a):

Troche to wygląda jakbyś sam nie wiedział co to ma robić.

Żeby zacząć od DDD, to musiałbyś wiedzieć jakie funkcje ma spełniać ta aplikacja - co ona ma robić?

Ma znaleźć wszystkie dostępne miejsca na jakiś film we wszystkich kinach, pokazać je userowi żeby ten mógł wybrać jedno, i potem kupić jakieś miejsce?

Bo jeśli tak, to raczej trochę to na około zrobiłeś.

Nie, to apka do jednego kinia

A co użytkownicy mieliby robić korzystając z tej aplikacji?

Co ma wgl wspólnego ta klasa z koncepcją DDD? Serio pytam, bo nie wiem w zamyśle o co chodzi autorowi

@Riddle: No rezerwować miejsca na seans w tym kinie? Trochę nie rozumiem, czemu o to pytasz. Myślałem, że to oczywiste

Problem jest taki, że to jest jedna klasa w izolacji.

Żeby gadać o DDD, warto byłoby pomyśleć (uzyskać tę informację albo chociaż zgadywać czy tworzyć hipotezy), w jaki sposób to będzie używane w całym systemie.

    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;
    }

Tego nie rozumiem.

1. bierzesz obiekt booking jako argument
2. wsadzasz go do środka agregatu
3. później robisz return booking, czyli zwracasz ten sam obiekt, który już powinien należeć do agregatu?

Pytanie, po co ci ten obiekt Booking, co z nim dalej robisz (i czy powinieneś cokolwiek robić).

Nofenak napisał(a):

@Riddle: No rezerwować miejsca na seans w tym kinie? Trochę nie rozumiem, czemu o to pytasz. Myślałem, że to oczywiste

Nie napiszesz tej aplikacji z DDD, jeśli sam nie wiesz jak ona ma działać.

Musisz sprecyzować, jakie dane ma widzieć użytkownik, co może z nimi zrobić, ile miejsc może zarezerwować, jak te dane będą się zmieniać w czasie, w jaki sposób pójdzie płatność, i wszystkie inne szczegóły.

DDD to nie struktura kodu a swego rodzaju metodyka jego wytwarzania w oparciu o rozmowy z biznesem (dosyć sporo tam się dzieje i ciężko to w kilku słowach opisać).

Jeśli chcesz popatrzeć na to od strony technicznej to rzuć okiem na:
https://github.com/ddd-by-examples

BTW:
jest jeszcze fajne video na yt wyjaśniające czym jest DDD (imho przydatne dla totalnego laika w temacie [w tym mnie]):

@Nofenak: to raczej wygląda na taki typowy serwis ze springa. Agregat w DDD musi spełniać kilka reguł, m.in. dostęp do encji agregatu może być zrealizowany jedynie przez korzeń. U ciebie mamy taki fragment:

    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;
    }

Agregat ujawnia swój wewnętrzny element, czyli booking, a w dodatku robi to w sposób całkowicie pozbawiający się kontroli. Zauważ, że KAŻDY inny obiekt w systemie, jeżeli dostanie booking to może zmienić jego stan. Podobnie ma się lista siedzeń. Przychodzi z zewnątrz i nie mamy kontroli nad jej zawartością. Inny obiekt może np. usunąć siedzenie, które jest zarezerwowane.

Czujesz koncepcję, ale wykonanie nie jest dobre.

Jak to pozmieniać?

Po pierwsze konstruktor:

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

Kopiujemy listę siedzeń, dzięki czemu mamy nad nią kontrolę. Co prawda nadal nie mamy kontroli nad samymi obiektami Seat, ale to zadanie dla ciebie.

Po drugie rezerwacja:

public BookingView addBooking(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 booking = new Booking();
	booking.markAsActive(this, foundSeat);
	this.bookings.add(booking);

	BookingView bookingView = new BookingView(booking);

	return bookingView;
}

Wywalamy rezerwację z listy parametrów. Nasz agregat operuje na wewnętrznej reprezentacji Booking, którą zarządza. Na zewnątrz wystawia jedynie BookingView (tak wiem brzydkie), które jest prostą klasą z danymi rezerwacji. Można trochę inaczej to ograć, tworząc rekord Booking, który będzie miał tylko za zadanie przechowywać podstawowe dane, a wewnętrznie operować na czymś w rodzaju:

record BookingStatus(Booking booking, boolean active){
   public void markAsActive(){}
   public void markAsInactive(){}
}

To też nie jest zbyt ładne. Clou polega na tym, żeby Booking nie wyciekał w sposób, który pozwala na jego zmianę poza agregatem.