Simple refactors

README.md

@@ -1,11 +1,12 @@
 # Scheduler-rs
 > *A university schedule solver written in Rust.*
 
+
 *Part of [yacs](https://yacs.io/)*
 
 [![Build Status](https://travis-ci.org/YACS-RCOS/Scheduler-rs.svg?branch=master)](
     https://travis-ci.org/YACS-RCOS/Scheduler-rs)
-    
+
 [![Documentation](https://img.shields.io/badge/docs-blue.svg)](
     https://yacs-rcos.github.io/Scheduler-rs/doc/scheduler/index.html)
 
@@ -22,3 +23,15 @@
     https://www.rust-lang.org/)
 
 
+### Contributing
+- Download rust
+- Make a fork
+- Clone repository with `git clone https://github.com/YOUR_USERNAME/Scheduler-rs.git`
+- Make edits
+- Open pull request
+
+### File Structure
+- `lib.rs` - External API
+- `solver.rs` - Does math to solve an arbitrary set of schedules
+- `model.rs` - Internal API's to objects in library
+

src/lib.rs

@@ -13,7 +13,7 @@
 pub use serde;
 #[macro_use]
 extern crate serde_derive;
-pub use serde::{Serialize, Deserialize};
+pub use serde::{Deserialize, Serialize};
 
 /// ## The model module
 /// This module contains the data model and structures for storing schedules

src/model.rs

@@ -1,4 +1,5 @@
-use std::cmp::{min, max};
+use std::cmp::PartialEq;
+use std::cmp::{max, min};
 
 /// Time type that we use, currently corresponds to seconds, or unix time.
 pub type TimeUnit = u64;
@@ -12,138 +13,159 @@ pub type TimeUnit = u64;
 /// - Fencing practice is an event.
 #[derive(Eq, Serialize, Deserialize, Clone, Debug, Hash)]
 pub struct Event {
+    /// Univserally unique id for this event
     pub uuid: String,
-    /// Offset from the start of the owning scheduleable.
+    /// Offset from the start of the owning schedule.
     pub offset: TimeUnit,
+    /// Duration of the event
     pub duration: TimeUnit,
     /// Time from start -> start of next event
     pub repeat: TimeUnit,
 }
 
-/// An option for a scheduleable.
+/// An option for a schedule.
 ///
-/// Ex. A course is a scheduleable. A section of that course is one of its
-/// scheduleable options.
+/// Ex. A course is a schedule. A section of that course is one of its
+/// options.
 #[derive(Clone, Eq, Serialize, Deserialize, Debug, Hash)]
-pub struct ScheduleableOption {
+pub struct ScheduleOption {
+    /// Universally unique id for this option
     pub uuid: String,
+    /// Events associated with this option
     pub events: Vec<Event>,
 }
 
 /// Something which can be scheduled.
 ///
 /// Ex:
-/// - A course is a scheduleable.
-/// - A a club is a scheduleable.
-/// - A sport is a scheduleable.
+/// - A course is a schedule.
+/// - A a club is a schedule.
+/// - A sport is a schedule.
 #[derive(Clone, Eq, Serialize, Deserialize, Debug, Hash)]
-pub struct Scheduleable {
+pub struct ScheduleList {
+    /// Universally unique id for this list
     pub uuid: String,
+    /// start time of first event of this schedule
     pub start: TimeUnit,
-    /// Duration of this scheduleable.
+    /// Duration of this schedule list.
     /// This should be at least the difference between the start of this
-    /// scheduleable and the end of the last event or event repeat.
+    /// schedule list and the end of the last event or event repeat.
     pub duration: TimeUnit,
-    pub options: Vec<ScheduleableOption>,
+    /// Options available in list
+    pub options: Vec<ScheduleOption>,
 }
 
-
-/// Labeled ScheduleabelOption, used in solver.
+/// Labeled ScheduleOption, used in solver.
 #[derive(Clone, Eq, PartialEq, Debug, Hash)]
-pub struct InfoScheduleableOption<'option_lifetime> {
-    pub inner: &'option_lifetime ScheduleableOption,
+pub struct InfoScheduleOption<'option_lifetime> {
+    /// Options wrapped by this struct
+    pub inner: &'option_lifetime ScheduleOption,
+    /// Start as defined by schedule list
     start: TimeUnit,
+    /// End as defined by schedule list's start + duration
     end: TimeUnit,
 }
 
-impl ::std::cmp::PartialEq for Scheduleable {
-    fn eq(&self, other: &Self) -> bool {other.uuid == self.uuid}
+impl PartialEq for ScheduleList {
+    fn eq(&self, other: &Self) -> bool {
+        other.uuid == self.uuid
+    }
 }
 
-impl ::std::cmp::PartialEq for ScheduleableOption {
-    fn eq(&self, other: &Self) -> bool {other.uuid == self.uuid}
+impl PartialEq for ScheduleOption {
+    fn eq(&self, other: &Self) -> bool {
+        other.uuid == self.uuid
+    }
 }
 
-impl ::std::cmp::PartialEq for Event {
-    fn eq(&self, other: &Self) -> bool {other.uuid == self.uuid}
+impl PartialEq for Event {
+    fn eq(&self, other: &Self) -> bool {
+        other.uuid == self.uuid
+    }
 }
 
-impl Scheduleable {
-    /// This scheduleables start + duration.
-    pub fn get_end(&self) -> TimeUnit {self.start + self.duration}
+impl ScheduleList {
+    /// This schedule's start + duration.
+    pub fn get_end(&self) -> TimeUnit {
+        self.start + self.duration
+    }
 
-    /// Labels all of the ScheduleableOptions in self.options
-    pub fn label_options(&self) -> Vec<InfoScheduleableOption> {
+    /// Labels all of the ScheduleOptions in self.options
+    pub fn label_options(&self) -> Vec<InfoScheduleOption> {
         self.options
             .iter()
-            .map(|scheduleable_option| {InfoScheduleableOption{
-                inner: scheduleable_option,
+            .map(|option| InfoScheduleOption {
+                inner: option,
                 start: self.start,
                 end: self.get_end(),
-            }})
+            })
             .collect()
     }
 
-    /// Returns true if none of the ScheduleableOptions conflict with themselves.
-    pub fn is_valid(&self) -> bool { self.label_options().iter().all(|iso| iso.is_valid()) }
-
+    /// Returns true if none of the ScheduleOptions conflict with themselves.
+    pub fn is_valid(&self) -> bool {
+        self.label_options().iter().all(|iso| iso.is_valid())
+    }
 }
 
-impl<'a> InfoScheduleableOption<'a> {
-
-    /// Check if two InfoSceduleableOptions conflict.
+impl<'a> InfoScheduleOption<'a> {
+    /// Check if two InfoSceduleOptions conflict.
     pub fn conflict(&self, other: &Self) -> bool {
         let start = max(self.start, other.start);
         let end = min(self.end, other.end);
-        if end < start {false}
-        else {
+        if end < start {
+            false
+        } else {
             let duration = end - start;
-            !self.inner.events.iter()
-                .any(|event| {
-                    other.inner.events.iter()
-                        .any(|event2| {
-                            event.contains_between(0, event2.offset, duration) ||
-                            event.contains_between(0, event2.get_end(), duration)
-                        })
+            !self.inner.events.iter().any(|event| {
+                other.inner.events.iter().any(|event2| {
+                    event.contains_between(0, event2.offset, duration)
+                        || event.contains_between(0, event2.get_end(), duration)
                 })
+            })
         }
     }
 
-    /// A ScheduleableOption is valid if none of its events conflict with each other.
+    /// A ScheduleOption is valid if none of its events conflict with each other.
     /// This is unfortunately an O(n^2) operation.
     fn is_valid(&self) -> bool {
-        self.inner.events.iter()
+        self.inner
+            .events
+            .iter()
             .map(|event| (event, &self.inner.events))
             .all(|(event, ref vec)| {
-                !vec.iter()
-                    .any(|event2| {
-                        event.contains_between(0, event2.offset, self.end-self.start) ||
-                        event.contains_between(0, event2.get_end(), self.end-self.start)
-                    })
+                !vec.iter().any(|event2| {
+                    event.contains_between(0, event2.offset, self.end - self.start)
+                        || event.contains_between(0, event2.get_end(), self.end - self.start)
+                })
             })
     }
 }
 
-
 impl Event {
     /// Check if this event or any of its repetitions within `during`
     /// contain `time`.
-    pub fn contains(&self, time: TimeUnit, during: &Scheduleable) -> bool {
+    pub fn contains(&self, time: TimeUnit, during: &ScheduleList) -> bool {
         self.contains_between(during.start, time, during.get_end())
     }
 
     /// This event's offset + duration.
-    pub fn get_end(&self) -> TimeUnit {self.offset + self.duration}
-
+    pub fn get_end(&self) -> TimeUnit {
+        self.offset + self.duration
+    }
 
+    /// Check if `time` happens during one of the repetitions of this event
     fn contains_between(&self, start: TimeUnit, time: TimeUnit, end: TimeUnit) -> bool {
-        let mut mut_start = self.offset + start;
-        while mut_start < end {
-            if mut_start <= time && mut_start + self.duration > time { return true }
-                else if self.repeat != 0 { mut_start += self.repeat; }
-                    else { return false }
+        let mut start = self.offset + start;
+        while start < end {
+            if start <= time && start + self.duration > time {
+                return true;
+            } else if self.repeat != 0 {
+                start += self.repeat;
+            } else {
+                return false;
+            }
         }
         false
     }
-
-}
+}

src/solver.rs

@@ -1,52 +1,59 @@
 use crate::model::*;
 
 /// Returns a vector of all the possible non-conflicting sets of schedules.
-pub fn solve(scheduleables: Vec<Scheduleable>) -> Vec<Vec<ScheduleableOption>> {
-    solve_rec(scheduleables.as_slice(), vec![])
+pub fn solve(schedules: Vec<ScheduleList>) -> Vec<Vec<ScheduleOption>> {
+    solve_rec(schedules.as_slice(), vec![])
         .iter()
-        .map(|v| v.iter()
-            .map(|iso| iso.inner.clone())
-            .collect())
+        .map(|v| v.iter().map(|iso| iso.inner.clone()).collect())
         .collect()
 }
 
 /// Recursively solve for all possible schedules.
-fn solve_rec<'lifetime>(scheduleables: &'lifetime [Scheduleable], current: Vec<InfoScheduleableOption<'lifetime>>)
-    -> Vec<Vec<InfoScheduleableOption<'lifetime>>> {
-    match scheduleables.len() {
+fn solve_rec<'lifetime>(
+    schedules: &'lifetime [ScheduleList],
+    current: Vec<InfoScheduleOption<'lifetime>>,
+) -> Vec<Vec<InfoScheduleOption<'lifetime>>> {
+    match schedules.len() {
         0 => vec![current],
         1 => {
+            // TODO Maybe this should just be a subroutine?
+            // Seems unnecessary to make it recursive, and we just incur more
+            // overhead from the match statement
             /*
-            Filter out any scheduleable option that conflict with anything in the current
-            list of scheduleable_options.
-            Push all the scheduleable options that don't conflict with the current schedule
+            Filter out any schedule option that conflict with anything in the current
+            list of schedule_options.
+            Push all the schedule options that don't conflict with the current schedule
             onto clones of the current schedule and return all of the modified clones.
             */
-            scheduleables.last()
+            schedules // TODO Maybe this could be simpler? i.e. do we need to label options every time?
+                .last()
                 .unwrap()
                 .label_options()
                 .iter()
-                .filter(|scheduleable_option| !current.iter()
-                    .any(|currently_selected_scheduleable_option|
-                        currently_selected_scheduleable_option.conflict(scheduleable_option)))
-                .map(|non_conflicting_scheduleable_option| {
+                .filter(|option| {
+                    !current
+                        .iter()
+                        .any(|current_option| current_option.conflict(option))
+                })
+                .map(|option| {
                     let mut vec = current.clone();
-                    vec.push(non_conflicting_scheduleable_option.clone());
+                    vec.push(option.clone());
                     vec
                 })
                 .collect()
-        },
+        }
         n => {
+            // TODO Is this correct?
             let mut possible_schedules = vec![];
             for i in 0..n {
-                let mut next_layer_of_possible_schedules = vec![];
+                // TODO Is this correct?
+                let mut next_layer = vec![];
                 for schedule in possible_schedules {
-                    let mut some_possible_schedules = solve_rec(&scheduleables[i..i+1], schedule);
-                    next_layer_of_possible_schedules.append(&mut some_possible_schedules);
+                    next_layer.append(&mut solve_rec(&schedules[i..i + 1], schedule));
                 }
-                possible_schedules = next_layer_of_possible_schedules;
+                possible_schedules = next_layer;
             }
             possible_schedules
-        },
+        }
     }
-}
+}