Adding a new Booking to a list of Bookings, each Booking has two LocalDates (checkin, checkout)

Question

So I'm working on a personal project, trying to learn Java 8 and Spring Boot. I'm developing a REST API where you can view and book accommodations.

These are my model classes, here's the Accommodation class:

@Data
@Document(collection = "accommodations")
public class Accommodation {

    @Id
    private String accommodationId;
    private Double pricePerNight;
    private Integer guests;
    private Address address;
    private Landlord landlord;
    private List<Review> reviews;
    private List<Booking> bookings;

    private Accommodation() {
        this.reviews = new ArrayList<>();
        this.bookings = new ArrayList<>();
    }

    public Accommodation(Double pricePerNight, Integer guests, Address address, Landlord landlord) {
        this();
        this.pricePerNight = pricePerNight;
        this.guests = guests;
        this.address = address;
        this.landlord = landlord;
    }

    public Boolean isAvailableBetween(LocalDate checkin, LocalDate checkout) {

        // TODO: fix

        boolean available = bookings
                .stream()
                .anyMatch(b ->
                        (checkin.isAfter(b.getCheckin()) || checkin.isEqual(b.getCheckin())) &&
                                (checkout.isBefore(b.getCheckout()) || checkout.isEqual(b.getCheckout()))
                );

        return !available;
    }

}

Here's the Booking class:

@Data
public class Booking {

    private String bookingId;
    private LocalDate checkin;
    private LocalDate checkout;
    private LocalDate bookedAt;

    private Booking(){
        this.bookedAt = LocalDate.now();
        this.bookingId = new ObjectId().toString();
    }

    public Booking(LocalDate checkin, LocalDate checkout) {
        this();
        this.checkin = checkin;
        this.checkout = checkout;
    }
}

Now, what I've been stuck with, and I would like to accomplish is to make sure that you can add a Booking to the List of bookings of an Accommodation, ONLY IF the new Booking's checkin and checkout LocalDates do not overlap any of the range of LocalDates of any other Booking in the same list.

TLDR; need to make sure that you can only book the Accommodation in the days that the Accommodation is available.

Here's the method in my Service class, which is called by a Controller after an HTTP POST request for a booking:

public BookingDto bookAccommodation(String accommodationId, BookingDto bookingDto) {

    // TODO: fix and improve

    if (this.isCheckoutDateInvalid(bookingDto)) {
        throw new InvalidRequestException("Invalid request.", Arrays.asList("Check Out must be after Check In."));
    }

    Optional<Accommodation> accommodation = this.accommodationRepository.findById(accommodationId);

    accommodation.orElseThrow(() -> new AccommodationNotFoundException(String.format("Accommodation not found for ID: {}", accommodationId)));

    return accommodation
            .filter(a -> a.isAvailableBetween(bookingDto.getCheckin(), bookingDto.getCheckout()))
            .map(a -> bookAccommodationInternal(bookingDto, a))
            .orElseThrow(() -> new AccommodationNotAvailableException(String.format("Accommodation already booked for ID: {}", accommodationId)));
}

private Boolean isCheckoutDateInvalid(BookingDto bookingDto) {
    return bookingDto.getCheckout().isBefore(bookingDto.getCheckin());
}

private BookingDto bookAccommodationInternal(BookingDto bookingDto, Accommodation accommodation) {
    Booking booking = this.accommodationMapper.toBooking(bookingDto);
    accommodation.getBookings().add(booking);
    log.info(String.format("New booking created for ID: {}", booking.getBookingId()));
    this.accommodationRepository.save(accommodation);
    BookingDto newBookingDto = new BookingDto(booking);
    return this.linkAssembler.addLinksToBooking(accommodation.getAccommodationId(), newBookingDto);
}

Here's the BookingDto class:

@Getter
@NoArgsConstructor
public class BookingDto extends ResourceSupport {

    private String bookingId;

    @NotNull(message = "Check In must not be null.")
    @FutureOrPresent(message = "Check In must not be a past date.")
    @JsonFormat(shape = JsonFormat.Shape.STRING,
            pattern = "dd-MM-yyyy")
    private LocalDate checkin;

    @NotNull(message = "Check Out must not be null.")
    @FutureOrPresent(message = "Check Out must not be a past date.")
    @JsonFormat(shape = JsonFormat.Shape.STRING,
            pattern = "dd-MM-yyyy")
    private LocalDate checkout;

    @JsonFormat(shape = JsonFormat.Shape.STRING,
            pattern = "dd-MM-yyyy")
    private LocalDate bookedAt;

    public BookingDto(Booking booking) {
        this.bookingId = booking.getBookingId();
        this.checkin = booking.getCheckin();
        this.checkout = booking.getCheckout();
        this.bookedAt = booking.getBookedAt();
    }
}

Now as you can see from the code, I tried to work with a Stream in the "isAvailableBetween" method, owned by the Accommodation class. The predicate that I'm using works in some cases:

For example if the list of bookings of an Accommodation (let's call it Acc1) has two bookings with the following LocalDates:

Booking1

Checkin: 10/10/2019 Checkout: 31/10/2019

Booking2

Checkin: 01/11/2019 Checkout: 09/11/2019

Adding a Booking3 with:

Checkin: 12/10/2019 Checkout: 25/10/2019

is not going to be possible.

But it is possible (wrongly) to add a Booking4 with:

Checkin: 12/10/2019 Checkout: 02/11/2019

Any suggestions on how I can fix this predicate and still use Java Streams?
Or is there any other way (perhaps a better way) to accomplish this sort of "booking system" altogether?

I'm also open to any kind of tips about the rest of the code that I've shared. Anything really. Trying my hardest to improve. :)

Thanks in advance.

Have you considered executing a query against the database checking whether there is any accomodation between the dates of the new booking? — Elias, Sep 09 '19 at 10:11

score 1 · Accepted Answer · answered Sep 09 '19 at 10:59

1

Your error is here:

            .anyMatch(b ->
                    (checkin.isAfter(b.getCheckin()) || checkin.isEqual(b.getCheckin())) &&
                            (checkout.isBefore(b.getCheckout()) || checkout.isEqual(b.getCheckout()))
            );

First, the variable name should be occupied, not available, which is the opposite of what you are checking. Second, I believe the correct condition is (not tested):

            .anyMatch(b -> b.getCheckin().isBefore(checkout)
                             && b.getCheckout().isAfter(checkin)
            );

I like to think of it the other way around: The accommodation is available if the booking is entirely before or entirely after the one we are proposing. So if either b.getCheckin() is on or after checkout or b.getCheckout() is before or on checkin. The condition in the above code is the negation of this, so checks whether the booking b conflicts with the proposed one.

answered Sep 09 '19 at 10:59

Ole V.V.

81,772
15
137
161

Hi! Thanks for your reply. Although this doesn't cover the following case: If there's already a booking with checkin 10/10/2019 checkout 20/10/2019, and I try to add a new booking with checkin 11/10/2019 checkout 25/10/2019, this new booking will be wrongly added. – Andrea Damiani Sep 09 '19 at 11:45
Huh? Then `b.getCheckin()` is 10 Oct, which is before 25 Oct, and `b.getCheckout()` is 20 Oct, which is after 11 Oct, so my code should detect them as overlapping. I believe that what you need is [Determine Whether Two Date Ranges Overlap](https://stackoverflow.com/questions/325933/determine-whether-two-date-ranges-overlap). – Ole V.V. Sep 09 '19 at 11:51
1

EDIT: this actually allows you to add bookings with the same day multiple times. Tried to add a booking twice, with checkin 29-10-2019 checkout 29-10-2019 and it worked both times. – Andrea Damiani Sep 09 '19 at 13:02
Oh, I had assumed that checkout would also be strictly after checkin (not on the same date), so a booking would be for at least one night. I probably hadn’t understood the requirements precisely. I hope you can adjust the code to your exact needs. – Ole V.V. Sep 09 '19 at 13:06

Mirko Brandt · Answer 2 · 2019-09-09T12:20:44.443

0

Add this to your booking class:

public boolean isDateInsideCheckedIn(LocalDate date) {
    return checkin.isBefore(date) && checkout.isAfter(date);
}

Replace your isAvailableBetween with this:

public boolean isAvailableBetween(LocalDate checkin, LocalDate checkout) {
    return bookings.stream()
        .noneMatch(b -> 
            (b.isDateInsideCheckedIn(checkin) || b.isDateInsideCheckedIn(checkout)) 
            || (b.getCheckin().equals(checkin) && b.getCheckout().equals(checkout)));
}

This will now check if neither checkin nor checkout are inside an existing booking and if checkin and checkout aren't exactly the same on an existing booking.

edited Sep 09 '19 at 12:20

answered Sep 09 '19 at 12:04

Mirko Brandt

455
3
8

This actually looks quite clean. Although the previously suggested code works perfectly I wanna give this a try. I wonder if performance-wise this choice is better than the anyMatch variant. – Andrea Damiani Sep 09 '19 at 12:56
I think it depends on how many bookings usually are inside an accommodation. You probably only feel any difference if it's a lot of entries. But in theory noneMatch would be more efficient as it stops checking as soon as it finds a match while anyMatch will run through the entire List before stopping. – Mirko Brandt Sep 09 '19 at 13:01
If the existing booking is from 12 to 19 October and a new booking from 5 to 26 October is requested? Neither checkin nor checkout are inside the dates of the existing booking nor equal to the existing checkin or checkout, so can this new booking be wrongly created? – Ole V.V. Sep 09 '19 at 13:04

Adding a new Booking to a list of Bookings, each Booking has two LocalDates (checkin, checkout)

2 Answers2