Return Result instead of Option for the compare function ?

[ShellCode33]

Hey, thanks a lot for this library !

I'm by no mean an expert in Rust, I'm a complete beginner, but I feel like it would be more "rusty" if the compare() function returned a Result<u8> instead of an Option<u8>.

So I was wondering what is the rationale behind this ?

I use anyhow to handle my errors and everytime I use ssdeep-rs I have to match its result like this:

let score = ssdeep::compare(self.fuzzy_hash.as_bytes(), other.fuzzy_hash.as_bytes());

match score {
    Some(score) => Ok(score as u64),
    None => Err(anyhow!("unable to compare ssdeep hashes")),
}

I wish I could take advantage of the ? operator to propagate the error, like this:

let score = ssdeep::compare(self.fuzzy_hash.as_bytes(), other.fuzzy_hash.as_bytes())?;

[s3rvac]

Hi, thank you for the report :slightly_smiling_face:. You have a very good point, and I definitely agree that using a Result instead of an Option is the more natural choice :+1:. Frankly, I wrote this library back in 2016 when I started learning Rust, and I am no longer able to recall why I used Option instead of Result :sweat_smile:. Anyway, let me remedy that. After this is done, I will release a new version and write here.

[s3rvac]

I have published a new release 0.5.0 containing the following two changes:

• The hash(), hash_from_file(), and compare() functions now return a Result instead of Option.
• The compare() function now accepts &str instead of &[u8] as hash() returns String, not bytes. This simplifies and unifies the library interface. Indeed, instead of passing hash.as_bytes(), one can just pass &hash.

Could you please try the release and let me know if everything works for you as expected?

[ShellCode33]

Works like a charm ! Thanks a lot

One small think though: I noticed you use i8 as return value, considering you are handling the case where ssdeep returns -1 and return an error instead, I thought it would make sense to return an u8 instead of a i8. But I'm nitpicking really, it's more than fine the way it is. Performance wise it would probably be even better to return a usize, even if that's negligible

[s3rvac]

Good idea :+1:. I have published a new release 0.6.0, which makes compare() return Result<u8> instead of Result<i8>.

Let me close the issue. However, if you have any other suggestions, feel free to let me know.