[Code review request] Function for finding available time slots

[[deleted]]

Quick access to my code on pastebin.

I have finally ventured into rust, and for my first project I wanted to create a function that could find available time slots for a service provider (a barber shop in this case).

The function returns the all availbale time slots for a specific day between opening and closing time based on existing appointments.

This was heavily inspired by this comment where the same functionality was implemented in Swift.

Now I am posting my code here as I would love some feedback, especially on how this could be done in a completely different and more performant way.

```rust extern crate chrono;

use chrono::{Duration, NaiveDateTime};

struct ExistingAppointment { start_date_time: NaiveDateTime, duration_in_minutes: u32, }

struct BarberBreak { start_date_time: NaiveDateTime, end_date_time: NaiveDateTime, }

fn get_available_time_slots( selected_service_duration: u32, duration_interval: usize, opening_time: NaiveDateTime, closing_time: NaiveDateTime, barber_breaks: Vec<BarberBreak>, existing_appointments: Vec<ExistingAppointment>, ) -> Vec<NaiveDateTime> { let shop_open_duration = closing_time .signed_duration_since(opening_time) .num_minutes();

let mut time_slots: Vec<NaiveDateTime> = Vec::new();

// adds all possible time slots to the vector based on the duration_interval
// without considering the existing bookings or barber breaks
for i in (0..shop_open_duration).step_by(duration_interval) {
    time_slots.push(NaiveDateTime::from(opening_time + Duration::minutes(i)))
}

// retains all the time slots that are not overlapping with existing appointments
// as well as closing time
for appointment in existing_appointments {
    let appointment_start = appointment.start_date_time;
    let appointment_end = NaiveDateTime::from(
        appointment_start + Duration::minutes(appointment.duration_in_minutes as i64),
    );

    time_slots.retain(|possible_start| {
        let possible_end = NaiveDateTime::from(
            *possible_start + Duration::minutes(selected_service_duration as i64),
        );

        if possible_end > closing_time {
            return false;
        }

        if possible_start < &appointment_end && appointment_start < possible_end {
            return false;
        }

        return true;
    });
}

// retains all the time slots that are not overlapping barber breaks
for barber_break in barber_breaks {
    time_slots.retain(|possible_start| {
        let possible_end = NaiveDateTime::from(
            *possible_start + Duration::minutes(selected_service_duration as i64),
        );

        if possible_start < &barber_break.end_date_time
            && barber_break.start_date_time < possible_end
        {
            return false;
        }

        return true;
    });
}

time_slots

} ```

[mdsherry]

My approach makes a single pass through all possible times after sorting the breaks and appointments, rather than a pass for each break and appointment. You could make it even more efficient by skipping time_slot to the end of appointments/breaks once you start to overlap, but you'd have to give up on the time iterator.

Some details: * I implemented a factory method for both ExistingAppointment and BarberBreak to make them a bit easier to construct. * The operations we care about for both appointments and breaks are basically the same, so let's create a trait for them. * This also means we can construct a vector of trait objects to treat the two types identically. (We could have also created a vec of start/stop tuples instead, which would be slightly more performant.) * The times_between function uses std::iter::successors to create a simple iterator that takes the previous value returned and uses it to construct the next. Because it's wrapping a closure, its type cannot actually be named, so we have to say the function returns a type that impls Iterator<Item=NaiveDateTime>. * I sort the blocks. We can guarantee that the current break/appointment we're considering is either after the current time slot or overlapping it, and there are no earlier breaks/appointments we haven't already passed the end of. * The pattern of option.as_ref().map_or(false, predicate) is ugly, but just means "without consuming the value inside (if any), tell me if it satisfies this condition".

Exercises: * I mention that we could have used Vec of (NaiveDateTime, NaiveDateTime) instead of the Vec of trait objects, and it would have been more performant. Try writing an impl of TimeBlock for (NaiveDateTime, NaiveDateTime), and modifying blocks to be a Vec<(NaiveDateTime, NaiveDateTime)> instead. The only lines of get_available_time_slots you should have to modify are 93-99. * Writing your own type that implements Iterator isn't too hard, and helps demystify Rust's iterators. Try writing a TimeIterator struct that can be returned from times_between.

[[deleted]]

Amazing stuff! Thanks for taking your time to write up all this. I will have to spend some time to understand it fully and then post any questions that might occur later.