Ballasi / num2words

Convert numbers like 42 to forty-two
https://crates.io/crates/num2words
Apache License 2.0
17 stars 9 forks source link

to_words panics on NAN #12

Closed xd009642 closed 12 months ago

xd009642 commented 1 year ago

As in the issue title the following code will panic:

if let Some(word) = Num2Words::parse(&word).and_then(|x| x.to_words().ok()) {
    println!("Word: {}", word);
}
thread 'main' panicked at /home/daniel/.cargo/registry/src/index.crates.io-6f17d22bba15001f/num2words-1.0.0/src/lang/en.rs:78:53:
called `Option::unwrap()` on a `None` value
stack backtrace:
   0: rust_begin_unwind
             at /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/std/src/panicking.rs:595:5
   1: core::panicking::panic_fmt
             at /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/core/src/panicking.rs:67:14
   2: core::panicking::panic
             at /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/core/src/panicking.rs:117:5
   3: core::option::Option<T>::unwrap
             at /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/core/src/option.rs:935:21
   4: num2words::lang::en::English::split_thousands
             at /home/daniel/.cargo/registry/src/index.crates.io-6f17d22bba15001f/num2words-1.0.0/src/lang/en.rs:78:28
   5: num2words::lang::en::English::int_to_cardinal
             at /home/daniel/.cargo/registry/src/index.crates.io-6f17d22bba15001f/num2words-1.0.0/src/lang/en.rs:106:29
   6: num2words::lang::en::English::float_to_cardinal
             at /home/daniel/.cargo/registry/src/index.crates.io-6f17d22bba15001f/num2words-1.0.0/src/lang/en.rs:159:33
   7: <num2words::lang::en::English as num2words::lang::lang::Language>::to_cardinal
             at /home/daniel/.cargo/registry/src/index.crates.io-6f17d22bba15001f/num2words-1.0.0/src/lang/en.rs:188:13
   8: num2words::num2words::Num2Words::to_words
             at /home/daniel/.cargo/registry/src/index.crates.io-6f17d22bba15001f/num2words-1.0.0/src/num2words.rs:274:33
Ballasi commented 12 months ago

Hey xd009642, sorry for the long delay, I was on an international flight and the jet-lag got me!

Nice catch, I've never thought on testing NAN, I'm investigating that right now and will make sure to add a test case for that.

xd009642 commented 12 months ago

No worries, it was very easy to work around in my code in the meantime, and 4 days is faster than my current issue turnaround :sweat_smile: : . Thanks for the quick fix

Ballasi commented 12 months ago

4 days is faster than my current issue turnaround

Well, you probably end up with more complex issues than "oh you should add a .is_nan() case" haha!