Newbie looking for some critique

Hey all, I am 3days old in Rust and am having quite a tough time coming to grips with the design patterns in the language. I am mocking up a very basic dummy CLI and would be greatful if someone could take a look the code and give me any critiques/improvements?

use clap::{arg, crate_authors, crate_version, value_parser, Command};
use log::{debug, error, info, log_enabled, warn};
use serde::{Deserialize, Serialize};
use std::path::PathBuf;

struct FooAppOptions {
    config: Option<PathBuf>,
    mount: Option<PathBuf>,
}

impl Default for FooAppOptions {
    fn default() -> Self {
        Self {
            config: Some(PathBuf::new()),
            mount: Some(PathBuf::new()),
        }
    }
}

#[derive(Debug, Serialize, Deserialize)]
enum ApiVersion {
    V1,
    V2,
}

#[derive(Debug, Serialize, Deserialize)]
struct Config {
    api: ApiVersion,
    mount: PathBuf,
}

fn main() {
    env_logger::init();

    let cmd = Command::new("foo")
        .version(crate_version!())
        .author(crate_authors!())
        .arg(
            arg!(--config <CONFIG> "Config file to use to initialize the application.")
                .value_parser(value_parser!(PathBuf)),
        )
        .arg(arg!(--mount <MOUNT> "Mount path override. If not specified, the value from config will be used.")
             .value_parser(value_parser!(PathBuf))
             .required(false)
        );

    let mut options = FooAppOptions::default();
    let matches = cmd.get_matches();

    // Config filepath
    match matches.get_one::<PathBuf>("config") {
        Some(cfg_path) => {
            let cfg_path_val: &str = cfg_path.as_path().to_str().unwrap();
            debug!(
                "Config path specified:  {}",
                cfg_path.as_path().to_str().unwrap()
            );
            let cfg_found = cfg_path.exists();
            match cfg_found {
                true => {
                    debug!("Config found:  {}", cfg_path_val);
                    options.config = Some(cfg_path.to_owned());
                }
                false => debug!("Could not find specified config:  {}", cfg_path_val),
            }
        }
        None => debug!("No config specified."),
    }

    // Mount
    match matches.get_one::<PathBuf>("mount") {
        Some(mnt_path) => match mnt_path.is_dir() {
            true => {
                debug!(
                    "Mount path specified:  {}",
                    mnt_path.as_path().to_str().unwrap()
                );
                options.mount = Some(mnt_path.to_owned());
            }
            false => debug!("No override specified for the mount path."),
        },
        None => debug!("No override specified for the mount path."),
    }

    // Some logic based on provided
    // Whats a better way of tackling the below??
    let cfg_data: Config = std::fs::read_to_string(options.config.unwrap())
        .map(|val| serde_json::from_str(val.as_str()).unwrap())
        .unwrap();
    debug!("{:?}", cfg_data);

    // Add some (imaginary :) ) overrides to the config
    {}
}

I have some background in C++/C and largely Python. I am finding it hard to tackle the flurry of match patterns I require to capture the inputs and do something meaningful with it. For instance in the above snippet, I am mocking up a CLI app which takes a config file and an optional mount path (if not provided, it will use the one in Config).

I cant see an easy way to tackle this in Rust where it is something very trivial in Python (I dont want to set false equivalences between the two. I realise they both have different strong suites)

Any guidance would be really appreciated!

Use #[derive(Parser)] for declaratively parsing arguments without boilerplate. You won't need to write a single line of code.

There's a lot of potentially excess debugging. Don't bother checking if an input file exists before trying to read it for one; that's a TOCTOU situation. Log output in general isn't necessarily human-friendly either.

The error chaining can get a bit nicer with some factoring into error-propogating methods and functions.

Here's where I got when applying suggestions so far, ignoring everything else.


Other thoughts:

  • Requiring an option for the --config is somewhat unusual. Consider a default path.
  • Consider an option for API version too.
  • Dying if the default path isn't there is also somewhat unusual. Consider not doing that.
    • This probably means an error if they didn't pass --mount either
    • You'd need a default API version (or an error if they didn't pass one)
2 Likes

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.