Search code examples
javaooprefactoringlaw-of-demeter

Law of demeter - using only one dot, could I improve this logic?


I have the following method:

    private boolean reserveSeat(int selectedRow, int selectedSeat) {
    if (show.getRows().get(selectedRow).getSeats().get(selectedSeat).getReservationStatus()) {
        return false;
    } else {
        show.getRows().get(selectedRow).getSeats().get(selectedSeat).reserve();
        setRowNumber(selectedRow);
        setSeatNumber(selectedSeat);

        return true;
    }
}

which resides in a Reservation class. This class has a Show Object (show), A show has Rows (another object), Rows have Seats (another object).

My question is could this method be improved? I have read about LoD and worried that my dot signals a bad design though I think it is logical. It is the Seat object that knows if it is reserved or not. However is going from Show to Seat talking to strangers? or is it ok because of the way each object contains the next object?

Apologies if my questing is not clear. What seems to happen with me (probably because I am self taught) is I design stuff that works then I read some OOP design principles and think crap, it works but it is not good design!

Any advice appreciated.


Solution

  • Yes, that chain of calls is way too long. If show is in charge of the seats, then it would be better if it's fully in charge. Right now it's not fully in charge, because seats can be reserved without the show's knowing. This fragmentation of responsibilities is strange.

    You can put show fully in charge by not exposing Seat to the Reservation, by hiding the seat status manipulations behind helper methods:

    private boolean reserveSeat(int selectedRow, int selectedSeat) {
        if (show.isSeatReserved(selectedRow, selectedSeat)) {
            return false;
        } else {
            show.reserveSeat(selectedRow, selectedSeat);
            setRowNumber(selectedRow);
            setSeatNumber(selectedSeat);
    
            return true;
        }
    }
    

    Or, if you don't want show to be in charge of the seats, then it should not be aware of the seats at all, so then you would access seats not through show, but another class that's in charge of that.