HiveBrain v1.2.0
Get Started
← Back to all entries
patternrustMinor

X-up utility for EVE Online

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
eveutilityforonline

Problem

I am learning Rust.

I also play EVE Online – a video game about internet spaceships. I decided that it would be fun to practice Rust by writing a simple utility to help me x up.


to x up (verb)



  • (EVE Online) To respond to a fleet commander with a list of ships one has available for combat.




All fleet compositions (a.k.a doctrines) have been obfuscated to protect the operational security of my character's alliance.

For example, if a fleet commander requests:


X-up for Shield Destroyers!

I might respond with:


x Svipul (default) / Burst

This indicates that I am available to pilot either a Svipul or a Burst (two specific kinds of internet spaceship) in the upcoming engagement.

Of course, I could look into my hangar and type that out manually, but I'm learning a new programming language so I may as well build an overkill utility.

Behavior

Source Code

Data File

I record my doctrines in YAML (the real file is a mite longer).

# //OPSEC// ALL DOCTRINES OBFUSCATED //OPSEC//

- name: Armor Battleships
  categories:
  - category: &tempests
      [
      # Tempest,
      ]
  - category: &hic
      [
      Onyx,
      ]
  - category: &ewar
      [
      Blackbird,
      Celestis,
      Maller,
      ]
  - category: &tackle
      [
      Sabre,
      Slasher,
      ]
- name: Heavy Armor
  categories:
  - category: &myrms
     [
     Myrmidon,
     ]
  - category: *ewar
  - category: *tackle
- name: Shield Destroyers
  categories:
  - category: &svipuls
      [
      Svipul (default),
      ]
  - category: &bursts
      [
      Burst,
      ]
  - category: *tackle

# //OPSEC// ALL DOCTRINES OBFUSCATED //OPSEC//


YAML anchors let me put common ship categories into multiple doctrines. When I accidentally blow up one of my ships, I simply comment out the name so that my x-up strings reflect my inventory.

Rust CLI

doctrine.rs

This parses my YAML file into Doctrine objects.

```
extern crate yaml_rust;
use self::yaml_rust::Yaml;

#[derive(D

Solution

main.rs

-
Filesystem paths should be represented using &Path or PathBuf. Paths are not just strings and don't even have to be UTF-8.

-
Become familiar with the methods on Option and Result; there's a lot of good ones to be able to use. For example, this is Option::expect, and Result::expect is similar:

match foo {
    Some(x) => x,
    None => panic!("a message"),
}


-
There's an unneeded turbofish (::<>) when collecting into a HashMap.

-
Instead of cloning the Yaml from the vector and then throwing the vector away, you can remove the item from the vector. In this case, you can use swap_remove because we don't care about the order of the rest of the vector.

-
Unless you are returning a non-zero exit code, I'd use return to exit from main.

-
There's an unneeded string creation to lookup in the HashMap.

extern crate clap;
extern crate yaml_rust;

use std::collections::HashMap;
use std::env::home_dir;
use std::fs::File;
use std::io::Read;
use std::path::{Path, PathBuf};

use clap::{App, Arg, SubCommand};

use self::yaml_rust::{Yaml, YamlLoader};

mod doctrine;

fn yaml_cfg_path() -> PathBuf {
    let yaml_cfg_relative_path = "Dropbox/Log/Xup/xup/src/xup.yaml";
    let mut home_dir = home_dir().expect("Could not resolve home directory.");
    home_dir.push(yaml_cfg_relative_path);
    home_dir
}

fn file_contents(path: &Path) -> String {
    let mut file = File::open(&path).expect("Can't open file");
    let mut contents = String::new();
    file.read_to_string(&mut contents).expect("Can't read file");
    contents
}

fn load_yaml_cfg() -> Yaml {
    let yaml_cfg_path = yaml_cfg_path();
    let file_contents = file_contents(&yaml_cfg_path);
    let mut contents = YamlLoader::load_from_str(&file_contents).expect("Couldn't load");

    let num_documents = contents.len();
    if num_documents > 1 {
        println!("{} documents found in {} ...", num_documents, yaml_cfg_path.display());
        println!("Reading doctrines from the first document and ignoring the rest.");
    }
    contents.swap_remove(0)
}

type DoctrineName = String;

fn load_doctrines() -> HashMap {
    let doctrines = doctrine::Doctrine::many_from_yaml(&load_yaml_cfg()).expect("Something");
    doctrines.into_iter().map(|d| (d.name.clone(), d)).collect()
}

fn ships(doctrine: &doctrine::Doctrine) -> Vec {
    doctrine.categories.iter().flat_map(|c| c.ships.iter().cloned()).collect()
}

fn xup(doctrine: &doctrine::Doctrine) -> String {
    "x ".to_string() + &ships(doctrine).join(" / ")
}

fn main() {
    let matches = App::new("xup")
        .about("Outputs x-up string for given doctrine")
        .arg(Arg::with_name("doctrine")
            .short("d")
            .long("doctrine")
            .value_name("DOCTRINE")
            .takes_value(true))
        .subcommand(SubCommand::with_name("ls").about("Lists available doctrines"))
        .get_matches();

    let doctrines = load_doctrines();

    if let Some(_) = matches.subcommand_matches("ls") {
        for doctrine_name in doctrines.keys() {
            println!("{}", doctrine_name);
        }
        return;
    }

    match matches.value_of("doctrine") {
        Some(doctrine_name) => {
            match doctrines.get(doctrine_name) {
                Some(doctrine) => println!("{}", xup(&doctrine)),
                None => println!("Requested doctrine {} not found.", doctrine_name),
            }
        }
        None => println!("No doctrine requested."),
    }
}


doctrine.rs

-
Usually, extern crate declarations are placed in the main.rs or lib.rs file. This avoids the need for the self qualifier.

-
My first overall thought is that there is a lot of rightward-drift. I'd extract a bunch of functions to help combat this. You also get the benefit of more places to add names.

-
The YAML library provides as_* methods that return an Option for when you only care about one type. This is quite handy with the next point.

-
Another cause of rightward-drift is the heavy use of the match statement, usually with one or two match arms. I'd advocate using a combination of the above as_* methods with the ok_or and and_then methods to have a more chained error handling.

-
Make a custom Result type alias. This helps ensure a consistent error type across your functions, allows you to change the error type more easily, and reduces the amount of typing you need to do.

-
There are many unneeded pub qualifiers. Don't expose more than you need to.

-
Generally, you can match on the dereferenced value of a type (match *foo) instead of cloning it. This is often used with the ref keyword (Some(ref value)). You can then clone the inner reference, instead of the entire matched object.

-
Prefer to use expect over unwrap. Expect allows you to have text in there that helps you find the offending line and maybe even suggests how to fix the issue.

-
Including one buried panic in code that generally returns `Result

Code Snippets

match foo {
    Some(x) => x,
    None => panic!("a message"),
}
extern crate clap;
extern crate yaml_rust;

use std::collections::HashMap;
use std::env::home_dir;
use std::fs::File;
use std::io::Read;
use std::path::{Path, PathBuf};

use clap::{App, Arg, SubCommand};

use self::yaml_rust::{Yaml, YamlLoader};

mod doctrine;

fn yaml_cfg_path() -> PathBuf {
    let yaml_cfg_relative_path = "Dropbox/Log/Xup/xup/src/xup.yaml";
    let mut home_dir = home_dir().expect("Could not resolve home directory.");
    home_dir.push(yaml_cfg_relative_path);
    home_dir
}

fn file_contents(path: &Path) -> String {
    let mut file = File::open(&path).expect("Can't open file");
    let mut contents = String::new();
    file.read_to_string(&mut contents).expect("Can't read file");
    contents
}

fn load_yaml_cfg() -> Yaml {
    let yaml_cfg_path = yaml_cfg_path();
    let file_contents = file_contents(&yaml_cfg_path);
    let mut contents = YamlLoader::load_from_str(&file_contents).expect("Couldn't load");

    let num_documents = contents.len();
    if num_documents > 1 {
        println!("{} documents found in {} ...", num_documents, yaml_cfg_path.display());
        println!("Reading doctrines from the first document and ignoring the rest.");
    }
    contents.swap_remove(0)
}

type DoctrineName = String;

fn load_doctrines() -> HashMap<DoctrineName, doctrine::Doctrine> {
    let doctrines = doctrine::Doctrine::many_from_yaml(&load_yaml_cfg()).expect("Something");
    doctrines.into_iter().map(|d| (d.name.clone(), d)).collect()
}

fn ships(doctrine: &doctrine::Doctrine) -> Vec<String> {
    doctrine.categories.iter().flat_map(|c| c.ships.iter().cloned()).collect()
}

fn xup(doctrine: &doctrine::Doctrine) -> String {
    "x ".to_string() + &ships(doctrine).join(" / ")
}

fn main() {
    let matches = App::new("xup")
        .about("Outputs x-up string for given doctrine")
        .arg(Arg::with_name("doctrine")
            .short("d")
            .long("doctrine")
            .value_name("DOCTRINE")
            .takes_value(true))
        .subcommand(SubCommand::with_name("ls").about("Lists available doctrines"))
        .get_matches();

    let doctrines = load_doctrines();

    if let Some(_) = matches.subcommand_matches("ls") {
        for doctrine_name in doctrines.keys() {
            println!("{}", doctrine_name);
        }
        return;
    }

    match matches.value_of("doctrine") {
        Some(doctrine_name) => {
            match doctrines.get(doctrine_name) {
                Some(doctrine) => println!("{}", xup(&doctrine)),
                None => println!("Requested doctrine {} not found.", doctrine_name),
            }
        }
        None => println!("No doctrine requested."),
    }
}
extern crate yaml_rust;

use self::yaml_rust::{yaml, Yaml};

#[derive(Debug, Clone, PartialEq)]
pub struct Doctrine {
    pub name: String,
    pub categories: Vec<Category>,
}

#[derive(Debug, Clone, PartialEq)]
pub struct Category {
    pub ships: Vec<String>,
}

pub type Result<T> = ::std::result::Result<T, &'static str>; //' just fixing the CR pretty-printer

fn load_name(doctrine: &yaml::Hash) -> Result<&str> {
    let name_key = Yaml::String("name".into());
    doctrine.get(&name_key)
        .ok_or("Doctrine name not a string.")
        .and_then(|name| {
            name.as_str().ok_or("Doctrine name not a string.")
        })
}

fn load_categories(doctrine: &yaml::Hash) -> Result<Vec<Category>> {
    let categories_key = Yaml::String("categories".into());
    doctrine.get(&categories_key)
        .ok_or("Doctrine has no categories.")
        .and_then(Category::many_from_yaml)
}

impl Doctrine {
    fn from_yaml(yaml: &Yaml) -> Result<Self> {
        yaml.as_hash()
            .ok_or("Expected doctrine.")
            .and_then(|doctrine| {
                let name = try!(load_name(&doctrine));
                let categories = try!(load_categories(&doctrine));

                Ok(Doctrine {
                    name: name.to_string(),
                    categories: categories,
                })
            })
    }

    pub fn many_from_yaml(doctrines: &Yaml) -> Result<Vec<Self>> {
        doctrines.as_vec()
            .ok_or("Expected list of doctrines.")
            .and_then(|doctrines| {
                doctrines.iter().map(Doctrine::from_yaml).collect()
            })
    }
}

fn load_ships(category_ships: &Yaml) -> Vec<String> {
    category_ships.as_vec()
        .map(|ships| {
            ships.iter().map(|ship| {
                let name = ship.as_str().expect("Ship name was not a string");
                String::from(name)
            }).collect()
        })
        .unwrap_or_else(Vec::new)
}

fn load_category(category: &yaml::Hash) -> Result<Category> {
    let category_key = Yaml::String("category".into());
    category.get(&category_key)
        .ok_or("Could not find category.")
        .map(|category_ships| {
            let ships = load_ships(&category_ships);
            Category { ships: ships }
        })
}

impl Category {
    fn from_yaml(yaml: &Yaml) -> Result<Self> {
        yaml.as_hash()
            .ok_or("Expected category.")
            .and_then(load_category)
    }

    fn many_from_yaml(categories: &Yaml) -> Result<Vec<Self>> {
        categories.as_vec()
            .ok_or("Expected list of categories.")
            .and_then(|categories| {
                categories.iter().map(Category::from_yaml).collect()
            })
    }
}

#[cfg(test)]
mod test {
    use super::*;
    use super::yaml_rust::{Yaml, YamlLoader};

    fn quick_parse(s: &str) -> Yaml {
        YamlLoader::load_from_str(&s).expect("Couldn't parse test YAML").swap_remove(0)
    }

    #[test]
    fn test_parse_categories() {
        l

Context

StackExchange Code Review Q#132687, answer score: 4

Revisions (0)

No revisions yet.