Don't panic

[hargoniX]

We should never panic, instead always return Result's and try not to unwrap unless we can be absolutely sure it is safe.

[iTranscend]

Hi @elpiel, there are only three panic!s in the codebase, all of which are part of the tests for the file_log_parser.

1. https://github.com/AeroRust/nmea/blob/931a6150433c00a3bc7bc9957e4a9f2969c2e7ed/tests/file_log_parser/mod.rs#L14-L15
2. https://github.com/AeroRust/nmea/blob/931a6150433c00a3bc7bc9957e4a9f2969c2e7ed/tests/file_log_parser/mod.rs#L75-L84
3. https://github.com/AeroRust/nmea/blob/931a6150433c00a3bc7bc9957e4a9f2969c2e7ed/tests/file_log_parser/mod.rs#L181

Is there a need to remove these ones?

[elpiel]

Hello @iTranscend thanks for taking a look at this! It's not needed to fix these places in the tests.

Apart from panic! macro calls, however, there are other methods that also panic on error. For example, Option and Result both have expect (if you know it's impossible to fail and you want to add a message to it), expect_err (mostly used for testing), unwrap and unwrap_err. There are more methods which do not panic but instead do something with the missing field (None) or failing check (Err).

Could you please take a look for unwrap & expect too in the code?

[elpiel]

I've also encountered a few unreachable! calls which should be best handled by an Error in case of bad data passed as a valid sentence for these fields.

Ideally, we should communicate the field name and the passed data which has triggered this error and eventually include e.g. valid options or even better - a message containing either the possible options or the expected data.