Simple command line parser/dispatcher in Rust

Question

I've written a very simple command line parser/dispatcher library. This is an extension of a previous review, which I've refactored, found here. Basically, a user can add functions to a Cmdr struct, then invoke them using a &str. I've made the library generic so that a user can define their own Result<T, E> to be returned from the functions added to Cmdr.

I appreciate any and all comments, especially those related to Rust best practices. One thing that slightly irks me is that I'm not sure that Cmdr should be living in the root lib.rs file. But, if I move it to say, cmdr.rs, my namespace would be something stupid like cmdr::cmdr::Cmdr, as my root directory is also named cmdr. Anyway, behold.

cmd.rs

pub struct Cmd<T, E> {
 pub name: String,
 pub invocation: Box<FnMut() -> Result<T, E>>,
}
impl<T, E> Cmd<T, E> {
 pub fn new<F: 'static + FnMut() -> Result<T, E>>(invoke_str: &str, invocation: F) -> Cmd<T, E> {
 Cmd {
 name: String::from(invoke_str),
 invocation: Box::new(invocation),
 }
 }
 pub fn invoke(&mut self) -> Result<T, E> {
 (self.invocation)()
 }
}

lib.rs

pub mod cmd;
use cmd::Cmd;
pub struct Cmdr<T, E> {
 cmds: Vec<Cmd<T, E>>,
}
impl<T, E> Cmdr<T, E> {
 pub fn new() -> Cmdr<T, E> {
 Cmdr { cmds: Vec::new() }
 }
 pub fn add<F: 'static + FnMut() -> Result<T, E>>(&mut self, name: &str, cmd: F) {
 self.cmds.push(Cmd::new(name, cmd));
 }
 pub fn invoke(&mut self, cmd_name: &str) -> Option<Result<T, E>> {
 let cmd_to_invoke = self.cmds.iter_mut().find(|cmd| cmd.name == cmd_name);
 if let Some(cmd) = cmd_to_invoke {
 Some(cmd.invoke())
 } else {
 None
 }
 }
}
mod tests {
 #[test]
 fn cmdr() {
 let mut cmdr: super::Cmdr<&str, &str> = super::Cmdr::new();
 cmdr.add("test1", || Ok("test1 executed."));
 cmdr.add("test2", || Ok("test2 executed."));
 // Good commands.
 let test1_msg = cmdr.invoke("test1").unwrap().expect("Error in test1.");
 assert!(test1_msg == "test1 executed.", "Incorrect test1 message.");
 let test2_msg = cmdr.invoke("test2").unwrap().expect("Error in test2.");
 assert!(test2_msg == "test2 executed.", "Incorrect test2 message.");
 // Bad command.
 if let Some(_) = cmdr.invoke("ghost") {
 assert!(false, "Non-existent command somehow returned Some().");
 }
 }
}

Some plans for the future are to...

• Enable the library to actually utilize FnMut closures. The framework is in place, but I currently don't have a strong enough grasp of lifetimes to implement this.

• Add sub-commands and command flags. It will be an interesting exercise in using the ? operator to propagate errors.

• Document the code in a Rusty style. I haven't delved in to code based doc tags, as of yet.

• Eventually, this will be a front end for a DHCP client/server package I'd like to implement in Rust. I may also release it on crates.io, if it matures into something worthwhile.

Answer 1

1. Prefer where clauses when your bounds get the tiniest bit complicated

2. An if let with an else is often better as a match.

3. If you match on an Option and return an Option, just use Option::map.

4. This allows you to remove the temporary variable and just chain functions

5. The tests module should have #[cfg(test)] to prevent it from even being compiled when not testing.

6. It's typical to use super::* in a test module.

7. It's better to let the compiler infer types instead of being explicit. This shows as an error because the compiler can't infer the error type. You could say cmdr: Cmdr<_, &str>, but it's better to actually exercise the error case.

8. Instead of unwrapping your success / failure, assert against them.

9. Use assert_eq instead of assert when comapring equality.

10. Use assert! and is_none instead of pattern matching

lib.rs

pub mod cmd;
use cmd::Cmd;
pub struct Cmdr<T, E> {
 cmds: Vec<Cmd<T, E>>,
}
impl<T, E> Cmdr<T, E> {
 pub fn new() -> Cmdr<T, E> {
 Cmdr { cmds: Vec::new() }
 }
 pub fn add<F>(&mut self, name: &str, cmd: F)
 where
 F: FnMut() -> Result<T, E> + 'static,
 {
 self.cmds.push(Cmd::new(name, cmd));
 }
 pub fn invoke(&mut self, cmd_name: &str) -> Option<Result<T, E>> {
 self.cmds.iter_mut()
 .find(|cmd| cmd.name == cmd_name)
 .map(|cmd| cmd.invoke())
 }
}
#[cfg(test)]
mod tests {
 use super::*;
 #[test]
 fn cmdr() {
 let mut cmdr = Cmdr::new();
 cmdr.add("test1", || Ok("test1 executed."));
 cmdr.add("test2", || Err(42));
 // Good commands.
 let test1_msg = cmdr.invoke("test1");
 assert_eq!(test1_msg, Some(Ok("test1 executed.")), "Incorrect test1 message.");
 let test2_msg = cmdr.invoke("test2");
 assert_eq!(test2_msg, Some(Err(42)));
 // Bad command.
 assert!(cmdr.invoke("ghost").is_none(), "Non-existent command somehow returned Some().");
 }
}

cmd.rs

pub struct Cmd<T, E> {
 pub name: String,
 pub invocation: Box<FnMut() -> Result<T, E>>,
}
impl<T, E> Cmd<T, E> {
 pub fn new<F>(invoke_str: &str, invocation: F) -> Cmd<T, E>
 where
 F: FnMut() -> Result<T, E> + 'static,
 {
 Cmd {
 name: String::from(invoke_str),
 invocation: Box::new(invocation),
 }
 }
 pub fn invoke(&mut self) -> Result<T, E> {
 (self.invocation)()
 }
}

---

I'm not sure that Cmdr should be living in the root lib.rs file.

In fact, I'd say the opposite is true: I'm not sure that Cmd belongs in it's own file. Unlike many languages, most Rust projects I've read have no hesitation in placing more than one type in a single file. I wouldn't think that Cmd carries enough weight to be separated.

There is some work underway to adjust Rust's module system, so the organization you've shown might return to vogue at some point.

my namespace would be something stupid like cmdr::cmdr::Cmdr, as my root directory is also named cmdr

You can use the façade pattern to re-export nested types at a higher module:

// lib.rs
pub use cmdr::Cmdr;

Option<Result<T, E>>

I'd encourage you to try this out a bit; my gut tells me that it won't be the most ergonomic to use.

Document the code in a Rusty style

Remember that the static type system provides a baseline of documentation and that function and argument names provide a bit more. Don't document the things that are obvious from the name or type of something. Consider making more types to improve the documentation and user experience.

Eventually, this will be a front end for a DHCP client/server package I'd like to implement in Rust. I may also release it on crates.io, if it matures into something worthwhile.

Please strongly consider not using your own hand-rolled library for such a project. Having a great DHCP server sounds wonderful, but one where the author's time has to be spent maintaining a command-line parsing library or one that has a substandard command line interface would detract from it.

Of course, that shouldn't stop you from using it to experiment with Rust!

• John Stritenberger: Yeah, as far as the DHCP server and Cmdr go, my primary objective is to learn. It would be cool if they matured into something publicly usable, but also, cool if they didn't. When I get into writing the DHCP stuff, I'll definitely shelve Cmdr if it's more of a maintenance burden than a learning tool. All in the pursuit of education and such. On a side note, you've put a fair amount of effort into reviewing my Rust posts, and I appreciate it.
• John Stritenberger: Also, bonus points if Err(42) refers to the Ultimate Answer.
• Shepmaster: @JohnStrit It's a very easy to identify number that's unlikely to collide with other numbers that occur more naturally (0, 1, -1) ^_^