Skip to content

Reduce amount of unwraps

Michael Aaron Murphy requested to merge hcpl:reduce-unwraps into master

Created by: hcpl

Things have done to achieve that:

  • Introducing an error_chain-ish custom error type, convertable from all other ones used in src/event.rs and src/cursor.rs.
  • Replacing .unwrap()s in src/event.rs and src/cursor.rs by explicit error return values.
  • Modifying public interfaces which is mentioned below.
  • Refactoring and fixing formatting.

Changes that can be considered as regressions:

  • Using custom error type instead of std::io::Error may add memory and likely but little CPU overhead, though compiler could possibly pull some null-pointer optimizations to completely remove the said overhead. This is also useful for end-users that want to filter the specific error which is hard with io::Error::new(/* whatever io::ErrorKind variant here */, "custom error message").
  • Breaking changes in the public cursor::DetectCursorPos trait: cursor_pos method now returns error::Result<(u16, u16)> instead of io::Result<(u16, u16)>.

Possible improvement ideas, their pros and cons:

  • Switch completely to error_chain - this introduces the backtrace dependency which may not be suitable for platforms that don't support it, and as a result, makes the error::Error type consume more memory - which was the primary reason to not to use that crate from the beginning. On the other hand, improved maintainability and readability.
  • If the breaking change above is accepted, use error::Error and error::Result throughout the crate, not just in src/event.rs and src/cursor.rs, for better interoperability and consistency (if make a breaking change anyway, do it correctly). If not, then convert everything back to io::Error and io::Result which is fine, but in this case custom errors do need heap allocation. And of course, no plans to go back to .unwrap()s (the fewer unexpected panics, the better).
Edited by Jeremy Soller

Merge request reports