Remove unnecessary unwrap()s

sogrim · Sep 17, 2022 · 3d5b5aa · 3d5b5aa
1 parent 6a86164
commit 3d5b5aa
Show file tree

Hide file tree

Showing 9 changed files with 38 additions and 27 deletions.
diff --git a/packages/server/src/config.rs b/packages/server/src/config.rs
@@ -22,7 +22,7 @@ impl Config<'_> {
     pub fn from_env() -> Self {
         Config {
             ip: &IP,
-            port: PORT.parse::<u16>().unwrap(),
+            port: PORT.parse::<u16>().expect("Failed to parse port"),
             uri: &URI,
             client_id: &CLIENT_ID,
             profile: &PROFILE,

diff --git a/packages/server/src/core/bank_rule/specialization_groups.rs b/packages/server/src/core/bank_rule/specialization_groups.rs
@@ -188,8 +188,7 @@ fn run_exhaustive_search(
         &courses,
         &mut best_match,
     )
-    .or(Some(best_match))
-    .unwrap() // unwraping is safe since the line above always returns Some(_)
+    .unwrap_or(best_match)
 }
 
 impl<'a> BankRuleHandler<'a> {

diff --git a/packages/server/src/core/degree_status/compute_bank.rs b/packages/server/src/core/degree_status/compute_bank.rs
@@ -65,15 +65,15 @@ impl<'a> DegreeStatusHandler<'a> {
                 sum_credit = bank_rule_handler.chain(chains, &mut chain_done);
                 completed = !chain_done.is_empty();
                 if completed {
-                    msg = Some(messages::completed_chain_msg(&chain_done));
+                    msg = Some(messages::completed_chain_msg(chain_done));
                 }
             }
             Rule::SpecializationGroups(specialization_groups) => {
                 sum_credit = bank_rule_handler
                     .specialization_group(specialization_groups, &mut groups_done_list);
                 completed = groups_done_list.len() >= specialization_groups.groups_number;
                 msg = Some(messages::completed_specialization_groups_msg(
-                    &groups_done_list,
+                    groups_done_list,
                     specialization_groups.groups_number,
                 ));
             }

diff --git a/packages/server/src/core/degree_status/preprocessing.rs b/packages/server/src/core/degree_status/preprocessing.rs
@@ -81,7 +81,9 @@ impl DegreeStatus {
         self.course_statuses.sort_by(|c1, c2| {
             c1.extract_semester()
                 .partial_cmp(&c2.extract_semester())
-                .unwrap() // unwrap can't fail because we compare only integers or "half integers" (0.5,1,1.5,2,2.5...)
+                .unwrap_or(std::cmp::Ordering::Equal)
+            // partial_cmp returns None if one of the two values are NaN, which should never happen
+            // still, to be on the safe side, we use Ordering::Equal in that case instead of unwrapping
         });
     }
 }
diff --git a/packages/server/src/core/messages.rs b/packages/server/src/core/messages.rs
@@ -55,29 +55,29 @@ pub fn missing_credit_msg(overflow: f32, from: &str, to: &str) -> String {
     }
 }
 
-pub fn completed_chain_msg(chain: &[String]) -> String {
+pub fn completed_chain_msg(mut chain: Vec<String>) -> String {
     let mut msg = "השלמת את השרשרת: ".to_string();
-    for course in chain {
-        if course == chain.last().unwrap() {
-            msg += course;
+    while let Some(course) = chain.pop() {
+        if chain.is_empty() {
+            msg += &course;
         } else {
             msg += &format!("{}, ", course);
         }
     }
     msg
 }
 
-pub fn completed_specialization_groups_msg(groups: &[String], needed: usize) -> String {
+pub fn completed_specialization_groups_msg(mut groups: Vec<String>, needed: usize) -> String {
     let mut msg = if groups.len() == ZERO as usize {
         "לא השלמת אף קבוצת התמחות".to_string()
     } else if groups.len() == SINGLE as usize {
         format!("השלמת קבוצת התמחות אחת (מתוך {}): ", needed)
     } else {
         format!("השלמת {} (מתוך {}) קבוצות התמחות: ", groups.len(), needed)
     };
-    for group in groups {
-        if group == groups.last().unwrap() {
-            msg += group;
+    while let Some(group) = groups.pop() {
+        if groups.is_empty() {
+            msg += &group;
         } else {
             msg += &format!("{}, ", group);
         }

diff --git a/packages/server/src/core/parser.rs b/packages/server/src/core/parser.rs
@@ -157,7 +157,7 @@ fn parse_course_status_pdf_format(line: &str) -> Result<(Course, Option<Grade>),
                 .rev()
                 .collect::<String>()
                 .parse::<f32>()
-                .unwrap();
+                .map_err(|e| AppError::Parser(format!("Bad Format: {}", e)))?;
             break;
         }
         index += 1;

diff --git a/packages/server/src/core/tests.rs b/packages/server/src/core/tests.rs
@@ -615,7 +615,9 @@ async fn test_overflow_credit() {
     );
     assert_eq!(
         degree_status.course_bank_requirements[5].message,
-        Some(messages::completed_chain_msg(&["פיסיקה 2פ'".to_string()]))
+        Some(messages::completed_chain_msg(
+            vec!["פיסיקה 2פ'".to_string()]
+        ))
     );
 
     assert_eq!(
@@ -709,7 +711,7 @@ async fn test_software_engineer_itinerary() {
     );
     assert_eq!(
         degree_status.course_bank_requirements[5].message,
-        Some(messages::completed_chain_msg(&[
+        Some(messages::completed_chain_msg(vec![
             "פיסיקה 2".to_string(),
             "פיסיקה 3".to_string()
         ]))

diff --git a/packages/server/src/db/services.rs b/packages/server/src/db/services.rs
@@ -115,8 +115,10 @@ macro_rules! impl_update {
                 )
                 .await
             {
-                // We can safely unwrap here thanks to upsert=true and ReturnDocument::After
-                Ok(item) => Ok(item.unwrap()),
+                Ok(item) => item.ok_or_else(|| {
+                    // This should never happen, but to avoid unwrapping we return an explicit error
+                    AppError::NotFound(format!("{}: {}", stringify!($db_item), id.to_string()))
+                }),
                 Err(err) => Err(AppError::MongoDriver(err.to_string())),
             }
         }

diff --git a/packages/server/src/resources/course.rs b/packages/server/src/resources/course.rs
@@ -103,13 +103,17 @@ impl CourseStatus {
     }
 
     pub fn extract_semester(&self) -> f32 {
-        match self.semester.clone() {
-            Some(semester) => {
-                let semester: Vec<&str> = semester.split('_').collect();
-                semester.last().unwrap().parse::<f32>().unwrap_or(0.0) //TODO explain
-            }
-            None => 0.0,
-        }
+        self.semester
+            .as_ref()
+            .map(|semester| {
+                semester
+                    .split('_')
+                    .last()
+                    .unwrap_or("0.0")
+                    .parse::<f32>()
+                    .unwrap_or(0.0)
+            })
+            .unwrap_or(0.0)
     }
 
     pub fn valid_for_bank(&self, bank_name: &str) -> bool {
@@ -206,7 +210,9 @@ impl<'de> Visitor<'de> for GradeStrVisitor {
             "פטור ללא ניקוד" => Ok(Grade::ExemptionWithoutCredit),
             "פטור עם ניקוד" => Ok(Grade::ExemptionWithCredit),
             "לא השלים" => Ok(Grade::NotComplete),
-            _ if v.parse::<u8>().is_ok() => Ok(Grade::Numeric(v.parse::<u8>().unwrap())),
+            _ if v.parse::<u8>().is_ok() => Ok(Grade::Numeric(
+                v.parse::<u8>().map_err(|e| Err::custom(e.to_string()))?,
+            )),
             _ => {
                 let err: E = Err::invalid_type(Unexpected::Str(v), &self);
                 log::error!("Json deserialize error: {}", err.to_string());