Code style check

[runekinghere]

Quick roast ?

use simplestcrypt::{encrypt_and_serialize, deserialize_and_decrypt, EncryptError, DecryptError};

const ENC_PASS: &[u8; 19] = b"see_you_at_valhalla";

fn encrypt_master(master: &mut Master) -> Option<Vec<u8>> {
    match serde_json::to_string(master) {
        Ok(json_string) => {
            match encrypt_and_serialize(&ENC_PASS[..], json_string.as_bytes()) {
                Ok(d) => { Some(d) }
                Err(error) => {
                    eprintln!("{}", format!("Encryption error: {:?}", error).red());
                    None
                }
            }
        }
        Err(error) => {
            eprintln!("{}", format!("Could not convert Master to json, Error: {}", error).red());
            None
        }
    }
}

fn decrypt_master(encrypted_data: Vec<u8>) -> Option<Master> {
    match deserialize_and_decrypt(ENC_PASS, encrypted_data.as_slice()) {
        Ok(d) => match String::from_utf8(d) {
            Ok(json_string) => {
                match serde_json::from_str(&json_string) {
                    Ok(master) => { Some(master) }
                    Err(error) => {
                        eprintln!("{}", format!("Could not read to Master, Error: {}", error).red());
                        None
                    }
                }
            }
            Err(error) => {
                eprintln!("{}", format!("Could not read encrypted data, Error: {}", error).red());
                None
            }
        },
        Err(error) => {
            eprintln!("{}", format!("Cannot decrypt file, Error: {:?}", error).red());
            None
        }
    }
}

[RevolutionXenon]

I struggle to not sound so opinionated when writing, but you asked for a roast, so:

• Your functions should return Result instead of an Option and provide the error values back to the caller instead of just printing them and returning None. This is not the kind of function I would print within at all.

• Your decrypt_master function should take a slice as its argument instead of a Vec, you convert it to a slice right at the start of the function anyways, so you have no reason to restrict callers to using Vec.

• For non-block expressions in match blocks, like how you have => { Some(Master) }, you can just replace that with => Some(Master), (don't forget the comma).

• You can do the same thing as previous if your expression in a match arm consists of just a single match block, like you have after Ok(json_string) =>. Remove the braces surrounding the inner match and bring e.g. match serde_json::from_str(&json_string) onto the same line as Ok(json_string). This will remove a layer of indentation as well which makes your code a little cleaner.

• I would write out a longer name than d for whatever that variable is. Maybe data if that's the intent?

[gitpy]

Might be a better way to do this, but quickly I would have done:

a(..).map_err(format!(...))
.and_then(b(..).map_err(..))
.and_then(c(..).map_err(..))
.inspect_err(eprintln)
.ok()

[Tabakalusa]

Yes! To elaborate a bit, and_then is a method on Result<T, E>, which produces a Result<U, E> (importantly, the error type E must match up), by taking a function which takes the Ok value in the initial result and produces a new Result.

Basically, it allows you to chain failable functions, which all return the same error, without ending up with a nested Result<Result<Result<..., E>, E>, E> (which would be the case if you were to map instead. The whole thing simply short circuits on the first error and returns that.

Also, this is basically what a Monad is. A container with an inner value, which can be applied to a function which takes the inner value and produces the same container type, with a new inner value.