From 7f1ff73fd02aa7e7d23dd30ad639d82efaa5547c Mon Sep 17 00:00:00 2001 From: Avi Gupta Date: Fri, 10 May 2024 17:53:40 -0500 Subject: [PATCH] Improve error handling, make some code more FP-esque --- Cargo.lock | 9 +++-- Cargo.toml | 1 + src/database.rs | 23 +++-------- src/lib.rs | 2 +- src/util.rs | 100 ++++++++++++++++++------------------------------ 5 files changed, 49 insertions(+), 86 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e3abeba..266376b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -245,6 +245,7 @@ dependencies = [ "reqwest", "serde", "serde_json", + "thiserror", "worker", ] @@ -1457,18 +1458,18 @@ dependencies = [ [[package]] name = "thiserror" -version = "1.0.57" +version = "1.0.60" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1e45bcbe8ed29775f228095caf2cd67af7a4ccf756ebff23a306bf3e8b47b24b" +checksum = "579e9083ca58dd9dcf91a9923bb9054071b9ebbd800b342194c9feb0ee89fc18" dependencies = [ "thiserror-impl", ] [[package]] name = "thiserror-impl" -version = "1.0.57" +version = "1.0.60" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a953cb265bef375dae3de6663da4d3804eee9682ea80d8e2542529b73c531c81" +checksum = "e2470041c06ec3ac1ab38d0356a6119054dedaea53e12fbefc0de730a1c08524" dependencies = [ "proc-macro2", "quote", diff --git a/Cargo.toml b/Cargo.toml index 9b586d7..5188304 100644 --- 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 index cd3fa6d..2429e9a 100644 --- a/src/database.rs +++ b/src/database.rs @@ -32,10 +32,7 @@ pub async fn fetch_results( .text() .await?; - let result_data: Vec = - serde_json::from_str(&body).map_err(|e| format!("JSON parsing error: {e}"))?; - - Ok(result_data) + Ok(serde_json::from_str::>(&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 = serde_json::from_str(&body)?; - - Ok(username_data + Ok(serde_json::from_str::>(&body)? .into_iter() .map(|user| user.username) .collect()) @@ -120,12 +115,7 @@ pub async fn fetch_podium_data(client: &Postgrest) -> Result, B .text() .await?; - let mut podium_data: Vec = - 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::>(&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 = - serde_json::from_str(&body).map_err(|e| format!("JSON parsing error: {e}"))?; + let all_times: Vec = serde_json::from_str(&body)?; let times_excluding_saturday: Vec = all_times .iter() @@ -193,9 +182,7 @@ pub async fn fetch_leaderboard_from_db( .text() .await?; - let mut leaderboard_data: Vec = - serde_json::from_str(&body).map_err(|e| format!("JSON parsing error: {e}"))?; - + let mut leaderboard_data: Vec = 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 index d414846..b609bb7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -105,7 +105,7 @@ async fn handle_user(ctx: &RouteContext, client: &Postgrest) -> Result 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, Vec) { - 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::() + / (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,7 +103,7 @@ 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( @@ -129,7 +111,7 @@ pub fn generate_scatter_plot_html( *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>, + all_user_entries: Vec<&mut [ResultEntry]>, ) -> Result> { 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 = vec![]; - let mut entries2: Vec = 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> = 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 = vec![]; - let mut entries2: Vec = vec![]; - let all_user_entries: Vec<&mut Vec> = 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" ); } }