Simple command-line program - can it be improved?

Hello All!
I am a Rust newbie and learning Rust since a month. Loving it so far. My daughter saw me learning Rust and she gave me this project to do :slight_smile:

I wanted to know if there is anyway I can improve this program. I am coming from a web dev (PHP) background.

Thanks in advance.

I made some changes which should be pretty simple here (it doesn't compile since colored isn't available on the playground).

I hoisted up the array declaration out of the loop, replaced the color strings with the color enum and used the rand crate more like it's supposed to, the gen method is generic and returns a bool here, the choose method comes from the SliceRandom trait. Also instead of reversing the vec at the end, I just reversed the iteration order.

plus tiny enhancement in the area of DRY principle... i'd replace this 5 line block:

if rng.gen() {
    result.push(format!("{}", c.to_uppercase().to_string().color(color)));
} else {
    result.push(format!("{}", c.to_lowercase().to_string().color(color)));
}

with these two lines:

let cased_char = if rng.gen() { c.to_uppercase() } else { c.to_lowercase() };
result.push(format!("{}", cased_char.to_string().color(color)));

so that you don't have to repeat whole result.push(...) line with tiny difference in the middle part.

2 Likes

char::to_lowercase and char::to_uppercase return different types ToLowercase and ToUppercase, add a to_string here to return the same type in both branches.

1 Like

You're technically correct. Best kind of correct! :slight_smile:
Joking aside, thank you for correction and apologies to post author for potential confusion.

    let cased_char = if rng.gen() {
        c.to_uppercase().to_string()
    } else {
        c.to_lowercase().to_string()
    };
    result.push(format!("{}", cased_char.color(color)));

the output after cargo format is not so nicely packed, and my line saving argument is void, but not repeating the result.push(...) command still helps a bit...

2 Likes

Thank you so much for your answers. I am going to try this out and learn all the new stuff...super!
A few questions:

  1. How does one learn to write better/efficient Rust code besides putting up the code for review (like I did right now)? Any tips will be helpful.
  2. Do the suggestions here improve performance? I know it's a really simple program but just for kicks, any way to check performance improvements? If so, how would you do that?

Thank you!

It’s difficult to predict how changes will affect performance. Modern compilers and computers have lots of behind-the-scenes optimizations that behave non-intuitively. In cases where it really matters, your best option is to write both versions and then use something like criterion to measure them.

1 Like

For me it's careful reading of the various documentation, the rust book, the cargo book, the nomicon, the standard library docs, the crates docs, etc. And then it's an accretion of all that over time combined with some "real world" programming experience with it.

Emphasizing "over time", there's no need to rush. For example, I probably know a lot (? shrug) about Rust, but I know next to nothing about async Rust and I've never actually used unsafe though I'm familiar with what they allow.

And also : clippy.

3 Likes

regarding performace, for some elementary checks, i drop in few lines like these:

let timer = std::time::Instant::now();

// TODO - code being tracked/performace-evaluated goes here between these two lines

let elapsed = timer.elapsed();

println!("elapsed time: {} ms", elapsed.as_millis());

(replace .as_millis() for other implemented methods depending on your granularity/scope needs)

Plus, do remember that "cargo build/run" by deafult runs debug build and can be a LOT slower than what is possible/real with Rust... :slight_smile:

Thanks. Trying this out but getting:

the trait bound colored::Color: std::convert::From<&colored::Color> is not satisfied. Seems to be an errors in:
result.push(format!("{}", cased_char.color(color)));

Trying to figure it out. I am new to Traits etc.

that's strange, it seems to work fine on my side :frowning:

paste few lines of the source (or ideally whole one in playground) where specifically the compiler error shows up...

rustc --version
rustc 1.52.1 (9bc8c42bb 2021-05-09)

full code dump:

use colored::{Color, Colorize};
use rand::{seq::SliceRandom, Rng};

fn main() {
    println!("Enter a word or a sentence --------->>>>>>");

    let colors = [
        Color::Red,
        Color::Green,
        Color::Yellow,
        Color::Blue,
        Color::Magenta,
        Color::Cyan,
    ];

    let mut word = String::new();
    std::io::stdin()
        .read_line(&mut word)
        .expect("Failed to read line");

    let mut result = Vec::new();
    let mut rng = rand::thread_rng();
    for c in word.chars().rev() {
        let color = *colors.choose(&mut rng).unwrap();
        let cased_char = if rng.gen() {
            c.to_uppercase().to_string()
        } else {
            c.to_lowercase().to_string()
        };
        result.push(format!("{}", cased_char.color(color)));
    }

    println!("The output is: {}", result.concat());
}

@logicman the copy here (note the *) is the difference between your two versions.

Good catch! Yes, that * was the issue. I don't know what it does but I have added it and it works now. Will read up on all the new things introduced in the code. This is good - I can see the difference between my code and this one.
Thank you all! Truly appreciate the help.

Here the * copies (cf the Copy trait) the chosen color, which you can see here implements Copy.

1 Like

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.