From 49f77c672ab683d35998c931db39a41075d0ab45 Mon Sep 17 00:00:00 2001 From: Josh Mitchell Date: Mon, 16 Nov 2020 14:01:42 +1100 Subject: [PATCH 1/3] Deref rather than clone in Frame::filter_coords() --- src/frame.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/frame.rs b/src/frame.rs index f8b6c50..79a06e3 100644 --- a/src/frame.rs +++ b/src/frame.rs @@ -40,14 +40,13 @@ impl Frame { } /// Filters the frame by removing all atoms not matching the given indeces. - pub fn filter_coords(self: &mut Frame, indeces: &[usize]) { + pub fn filter_coords(self: &mut Frame, indices: &[usize]) { self.coords = self .coords .iter() - .map(|elem| elem.clone()) .enumerate() - .filter(|&(i, _)| indeces.contains(&i)) - .map(|(_, elem)| elem) + .filter(|(i, _)| indices.contains(i)) + .map(|(_, elem)| *elem) .collect(); } From 8ce523cb68189004027193ad724a6843174b5df9 Mon Sep 17 00:00:00 2001 From: Josh Mitchell Date: Mon, 16 Nov 2020 14:06:01 +1100 Subject: [PATCH 2/3] Explicit returns for error cases --- src/iterator.rs | 2 +- src/lib.rs | 24 ++++++++++-------------- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/src/iterator.rs b/src/iterator.rs index 201dcf8..ab5519b 100644 --- a/src/iterator.rs +++ b/src/iterator.rs @@ -49,7 +49,7 @@ impl TrajectoryIterator { // It's OK to do this every frame because the result is cached by Trajectory let num_atoms = match &self.trajectory.get_num_atoms() { &Ok(n) => n, - Err(e) => Err(Error::CouldNotCheckNAtoms(Box::new(e.clone())))?, + Err(e) => return Err(Error::CouldNotCheckNAtoms(Box::new(e.clone()))), }; // Reuse old frame diff --git a/src/lib.rs b/src/lib.rs index e296061..6a916f5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -175,7 +175,7 @@ impl XDRFile { }) } else { // Something went wrong. But the C api does not tell us what - Err((path, filemode))? + Err((path, filemode).into()) } } } @@ -275,8 +275,8 @@ impl Trajectory for XTCTrajectory { .get_num_atoms() .map_err(|e| Error::CouldNotCheckNAtoms(Box::new(e)))?; if num_atoms != frame.coords.len() { - Err((&*frame, num_atoms))?; - }; + return Err((&*frame, num_atoms).into()); + } unsafe { let code = xdrfile_xtc::read_xtc( @@ -402,7 +402,7 @@ impl Trajectory for TRRTrajectory { .get_num_atoms() .map_err(|e| Error::CouldNotCheckNAtoms(Box::new(e)))?; if num_atoms != frame.coords.len() { - Err((&*frame, num_atoms))?; + return Err((&*frame, num_atoms).into()); } unsafe { @@ -631,21 +631,17 @@ mod tests { Ok(s) => { assert_eq!(s, CString::new("test")?); } - Err(_) => panic!("Valid Path failed to convert to CString.") + Err(_) => panic!("Valid Path failed to convert to CString."), } - // \0 in path should result in an InvalidOsStr(Some(NulError)) + // \0 in path should result in an InvalidOsStr(Some(NulError)) let result = path_to_cstring(PathBuf::from("invalid/\0path")); match result { Ok(_) => panic!("Cstring conversion did not fail"), - Err(e) => { - match e { - Error::InvalidOsStr(opt) => { - assert!(opt.is_some()) - } - _ => panic!("Wrong error type. (This should never happend).") - } - } + Err(e) => match e { + Error::InvalidOsStr(opt) => assert!(opt.is_some()), + _ => panic!("Wrong error type. (This should never happend)."), + }, } Ok(()) } From daccb110db801bf396e09ffa5595d8316bd5484f Mon Sep 17 00:00:00 2001 From: Josh Mitchell Date: Mon, 16 Nov 2020 14:19:57 +1100 Subject: [PATCH 3/3] ErrorCode::is_eof() now takes self by value (8 bits) rather than reference (64 bits) --- src/errors.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/errors.rs b/src/errors.rs index a5914e3..4986419 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -64,7 +64,7 @@ impl std::error::Error for Error { } else { None } - }, + } Error::CouldNotCheckNAtoms(err) => Some(err.as_ref()), _ => None, } @@ -200,7 +200,7 @@ pub enum ErrorCode { impl ErrorCode { /// True if the error is an end of file error, false otherwise - pub fn is_eof(&self) -> bool { + pub fn is_eof(self) -> bool { matches!(self, Self::ExdrEndOfFile) } }