Improve error handling, make some code more FP-esque

aviguptatx · May 10, 2024 · 7f1ff73 · 7f1ff73
1 parent b589088
commit 7f1ff73
Show file tree

Hide file tree

Showing 5 changed files with 49 additions and 86 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -21,6 +21,7 @@ dotenv = "0.15.0"
 chrono = "0.4.33"
 plotly = "0.8.4"
 reqwest = "0.11.24"
+thiserror = "1.0.60"
 
 [profile.release]
 lto = "fat"

diff --git a/src/database.rs b/src/database.rs
@@ -32,10 +32,7 @@ pub async fn fetch_results(
         .text()
         .await?;
 
-    let result_data: Vec<ResultEntry> =
-        serde_json::from_str(&body).map_err(|e| format!("JSON parsing error: {e}"))?;
-
-    Ok(result_data)
+    Ok(serde_json::from_str::<Vec<ResultEntry>>(&body)?)
 }
 
 /// Fetches the most recent crossword date from the database.
@@ -93,9 +90,7 @@ pub async fn fetch_usernames_sorted_by_elo(
         .text()
         .await?;
 
-    let username_data: Vec<UsernameData> = serde_json::from_str(&body)?;
-
-    Ok(username_data
+    Ok(serde_json::from_str::<Vec<UsernameData>>(&body)?
         .into_iter()
         .map(|user| user.username)
         .collect())
@@ -120,12 +115,7 @@ pub async fn fetch_podium_data(client: &Postgrest) -> Result<Vec<ResultEntry>, B
         .text()
         .await?;
 
-    let mut podium_data: Vec<ResultEntry> =
-        serde_json::from_str(&body).map_err(|e| format!("JSON parsing error: {e}"))?;
-
-    podium_data.truncate(10);
-
-    Ok(podium_data)
+    Ok(serde_json::from_str::<Vec<ResultEntry>>(&body)?[..10].to_vec())
 }
 
 /// Fetches the user data for a given username from the database.
@@ -152,8 +142,7 @@ pub async fn fetch_user_data(
         .text()
         .await?;
 
-    let all_times: Vec<ResultEntry> =
-        serde_json::from_str(&body).map_err(|e| format!("JSON parsing error: {e}"))?;
+    let all_times: Vec<ResultEntry> = serde_json::from_str(&body)?;
 
     let times_excluding_saturday: Vec<ResultEntry> = all_times
         .iter()
@@ -193,9 +182,7 @@ pub async fn fetch_leaderboard_from_db(
         .text()
         .await?;
 
-    let mut leaderboard_data: Vec<LeaderboardEntry> =
-        serde_json::from_str(&body).map_err(|e| format!("JSON parsing error: {e}"))?;
-
+    let mut leaderboard_data: Vec<LeaderboardEntry> = serde_json::from_str(&body)?;
     leaderboard_data.sort_by(|a, b| b.elo.partial_cmp(&a.elo).unwrap_or(Ordering::Equal));
 
     Ok(leaderboard_data)

diff --git a/src/lib.rs b/src/lib.rs
@@ -105,7 +105,7 @@ async fn handle_user<T>(ctx: &RouteContext<T>, client: &Postgrest) -> Result<Res
             username,
             scatter_plot_html,
             box_plot_html,
-            top_times: data.all_times.iter().take(3).cloned().collect(),
+            top_times: data.all_times[..3].to_vec(),
         }
         .render()
         .unwrap(),

diff --git a/src/util.rs b/src/util.rs
@@ -5,32 +5,21 @@ use plotly::layout::{Axis, RangeSelector, RangeSlider, SelectorButton, SelectorS
 use plotly::{BoxPlot, Layout, Plot, Scatter};
 use std::cmp::{max, min};
 use std::error::Error;
-use std::fmt;
 
 use crate::models::{NytApiResponse, NytResultEntry, ResultEntry};
 
-/// Custom error for plotting operations.
-#[derive(Debug, Clone)]
-struct PlottingError {
-    message: String,
-}
+use thiserror::Error;
 
-impl PlottingError {
-    fn new(message: &str) -> Self {
-        Self {
-            message: message.to_string(),
-        }
-    }
+#[derive(Debug, Error)]
+pub enum PlottingError {
+    #[error("Plotting error: User doesn't have enough entries to generate plot")]
+    NotEnoughEntries,
+    #[error("Plotting error: Couldn't find minimum moving average")]
+    MinMovingAverageNotFound,
+    #[error("Plotting error: Couldn't find maximum moving average")]
+    MaxMovingAverageNotFound,
 }
 
-impl fmt::Display for PlottingError {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "Plotting error: {}", self.message)
-    }
-}
-
-impl std::error::Error for PlottingError {}
-
 /// Computes the moving average for a given slice of `ResultEntry` values.
 ///
 /// # Arguments
@@ -47,21 +36,21 @@ fn compute_moving_averages(
     interval: usize,
     include_partial: bool,
 ) -> (Vec<String>, Vec<i32>) {
-    let mut dates = Vec::new();
-    let mut moving_averages = Vec::new();
-
-    for i in 0..entries.len() {
-        if include_partial || i >= interval - 1 {
+    entries
+        .iter()
+        .enumerate()
+        .filter(|(i, _)| include_partial || *i >= interval - 1)
+        .map(|(i, entry)| {
             let start = i.saturating_sub(interval - 1);
-            let end = i;
-            let sum: i32 = entries[start..=end].iter().map(|entry| entry.time).sum();
-            let average = sum / (end - start + 1) as i32;
-            dates.push(entries[i].date.clone());
-            moving_averages.push(average);
-        }
-    }
-
-    (dates, moving_averages)
+            let end = i + 1;
+            let average = entries[start..end]
+                .iter()
+                .map(|entry| entry.time)
+                .sum::<i32>()
+                / (end - start) as i32;
+            (entry.date.clone(), average)
+        })
+        .unzip()
 }
 
 /// Computes the average time for a given slice of `ResultEntry` values.
@@ -97,16 +86,9 @@ pub fn generate_scatter_plot_html(
         .iter()
         .map(|user_entries| user_entries.len())
         .min()
-        .ok_or_else(|| {
-            PlottingError::new("User doesn't have enough entries to generate scatter plot")
-        })?;
-
+        .ok_or(PlottingError::NotEnoughEntries)?;
     let include_partial = match min_user_entries_length {
-        0 => {
-            return Err(Box::new(PlottingError::new(
-                "User doesn't have enough entries to generate scatter plot",
-            )))
-        }
+        0 => return Err(Box::new(PlottingError::NotEnoughEntries)),
         1..=60 => true,
         _ => false,
     };
@@ -121,15 +103,15 @@ pub fn generate_scatter_plot_html(
             *times
                 .iter()
                 .min()
-                .ok_or_else(|| PlottingError::new("Couldn't find min moving average"))?,
+                .ok_or(PlottingError::MinMovingAverageNotFound)?,
         );
 
         max_moving_average = max(
             max_moving_average,
             *times
                 .iter()
                 .max()
-                .ok_or_else(|| PlottingError::new("Couldn't find max moving average"))?,
+                .ok_or(PlottingError::MaxMovingAverageNotFound)?,
         );
 
         let trace_times = Scatter::new(dates.clone(), times)
@@ -195,25 +177,21 @@ pub fn generate_scatter_plot_html(
 ///
 /// A `Result` containing the HTML string for the box plot, or a `PlottingError` if an error occurs.
 pub fn generate_box_plot_html(
-    all_user_entries: Vec<&mut Vec<ResultEntry>>,
+    all_user_entries: Vec<&mut [ResultEntry]>,
 ) -> Result<String, Box<dyn Error>> {
     let max_average_time = all_user_entries
         .iter()
         .filter(|user_entries| !user_entries.is_empty())
         .map(|user_entries| compute_average_time(user_entries))
         .max()
-        .ok_or_else(|| {
-            PlottingError::new("User doesn't have enough entries to generate box plot")
-        })?;
+        .ok_or(PlottingError::NotEnoughEntries)?;
 
     let mut plot = Plot::new();
 
     for user_entries in all_user_entries {
         let username = user_entries
             .first()
-            .ok_or_else(|| {
-                PlottingError::new("User doesn't have enough entries to generate box plot")
-            })?
+            .ok_or(PlottingError::NotEnoughEntries)?
             .username
             .clone();
 
@@ -379,50 +357,46 @@ mod tests {
         assert!(result.is_err());
         assert_eq!(
             result.err().unwrap().to_string(),
-            "Plotting error: User doesn't have enough entries to generate scatter plot"
+            "Plotting error: User doesn't have enough entries to generate plot"
         );
     }
 
     #[test]
     fn test_generate_scatter_plot_html_with_empty_user_entries() {
-        let mut entries1: Vec<ResultEntry> = vec![];
-        let mut entries2: Vec<ResultEntry> = vec![];
-        let all_user_entries: Vec<&mut [ResultEntry]> = vec![&mut entries1, &mut entries2];
+        let all_user_entries: Vec<&mut [ResultEntry]> = vec![&mut [], &mut []];
 
         let result = generate_scatter_plot_html(all_user_entries);
 
         assert!(result.is_err());
         assert_eq!(
             result.err().unwrap().to_string(),
-            "Plotting error: User doesn't have enough entries to generate scatter plot"
+            "Plotting error: User doesn't have enough entries to generate plot"
         );
     }
 
     #[test]
     fn test_generate_box_plot_html_with_no_user_entries() {
-        let all_user_entries: Vec<&mut Vec<ResultEntry>> = vec![];
+        let all_user_entries: Vec<&mut [ResultEntry]> = vec![];
 
         let result = generate_box_plot_html(all_user_entries);
 
         assert!(result.is_err());
         assert_eq!(
             result.err().unwrap().to_string(),
-            "Plotting error: User doesn't have enough entries to generate box plot"
+            "Plotting error: User doesn't have enough entries to generate plot"
         );
     }
 
     #[test]
     fn test_generate_box_plot_html_with_empty_user_entries() {
-        let mut entries1: Vec<ResultEntry> = vec![];
-        let mut entries2: Vec<ResultEntry> = vec![];
-        let all_user_entries: Vec<&mut Vec<ResultEntry>> = vec![&mut entries1, &mut entries2];
+        let all_user_entries: Vec<&mut [ResultEntry]> = vec![&mut [], &mut []];
 
         let result = generate_box_plot_html(all_user_entries);
 
         assert!(result.is_err());
         assert_eq!(
             result.err().unwrap().to_string(),
-            "Plotting error: User doesn't have enough entries to generate box plot"
+            "Plotting error: User doesn't have enough entries to generate plot"
         );
     }
 }