3
\$\begingroup\$

Just on a chapter 3 of Rust Book so not much good in Rust. so tried to implement a simple temprature calculator as suggest in this chapter. so i would like you guys to point anything you feel

use std::io;

fn main() {
    println!("Enter Temperature as 56f or 98c");

    let mut usr_inpt = String::new();

    io::stdin()
        .read_line(&mut usr_inpt)
        .expect("can not read user input");

    let usr_inpt = usr_inpt.trim_end().to_lowercase();

    if usr_inpt.ends_with("f") || usr_inpt.ends_with("c") {
        // remove the last indication 56f -> 56 or 56c -> 56
        let _temp: String = usr_inpt.chars().take(usr_inpt.len() - 1).collect();
        let num: u32 = match _temp.parse() {
            Ok(num) => num,
            Err(_) => 0,
        };

        if usr_inpt.ends_with("f") {
            println!("celcius -> {}", (num - 32) * 5 / 9);
        } else if usr_inpt.ends_with("c") {
            println!("farenheit -> {}", num * 9 / 5 + 32);
        }
    } else {
        println!("invalid input");
    }
}
\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review. I've rolled your question back to its previous state, as it invalidated an already existing review. Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. \$\endgroup\$ – Zeta Jan 6 '19 at 7:59
  • 1
    \$\begingroup\$ my bad sure will take care of it ... \$\endgroup\$ – MAK Jan 6 '19 at 8:05
  • \$\begingroup\$ Please take a look at codereview.stackexchange.com/questions/207481/… \$\endgroup\$ – hellow Jan 9 '19 at 8:03
2
\$\begingroup\$
use std::io;

fn main() {
    println!("Enter Temperature as 56f or 98c");

    let mut usr_inpt = String::new();

    io::stdin()
        .read_line(&mut usr_inpt)
        .expect("can not read user input");

I like that you’re using expect here instead of unwrap. It allows for a much nicer user experience when things go sideways. If I was nitpicking, I’d remind you to use proper capitalization and punctuation.

    let usr_inpt = usr_inpt.trim_end().to_lowercase();

    if usr_inpt.ends_with("f") || usr_inpt.ends_with("c") {
        // remove the last indication 56f -> 56 or 56c -> 56
        let _temp: String = usr_inpt.chars().take(usr_inpt.len() - 1).collect();

temp is rarely a good name. If you meant temperature instead of temporary, it would be good to spell it out.

        let num: u32 = match _temp.parse() {
            Ok(num) => num,
            Err(_) => 0,
        };

Is it really the right thing to return zero in the error case? I would expect the error case here to be caused by invalid user input (or a bug in your previous parsing logic maybe). It would probably be best to alert the user to this failure instead of the slightly enigmatic result of 0.

        if usr_inpt.ends_with("f") {
            println!("celcius -> {}", (num - 32) * 5 / 9);
        } else if usr_inpt.ends_with("c") {
            println!("farenheit -> {}", num * 9 / 5 + 32);
        }

It smells a little funny that we’re making the same checks again as we did above. If you extracted the parsing logic into a function, you could just call it for each case. I would probably also extract proper fahrenheit_to_celcius and celcius_to_fahrenheit functions.

    } else {
        println!("invalid input");
    }
}

Error messages should be printed to stderr. Use the eprintln macro instead.

———————

All in all it’s pretty good for a first go. I would take a look at using some types for Fahrenheit and Celsius though. Your parse method could return a Temperature that contains either a Celsius or Fahrenheit measurement.

I’ve not run this through the compiler, but hopefully it illustrates the idea.

struct Celsius { value: u32 }
impl Celsius {
    fn to_farhenheit(&self) -> Farhenheit {
        Fahrenheit { value: self.value * 9 / 5 + 32 }
    }
}

struct Fahrenheit { value: u32 }
impl Fahrenheit {
    fn to_celsius(&self) -> Celsius {
        Celsius { value: ( self.value  - 32) * 5 / 9 }
    }
}

enum Temperature {
    Fahrenheit(Fahrenheit),
    Celsius(Celsius)
    Err(String)
}

fn parse_input(input: &str) -> Temperature {
    if input.ends_with("f") {
        Fahrenheit { value: parse_num(input) }
    } else if input.ends_with(“c”) {
        Celsius { value: parse_num(input) }
    } else {
        Err(“Input invalid. Must end with ‘c’ or ‘f’.”)
    }
}

fn parse_num(input: &str) -> u32 {
    let temperature: String = usr_inpt.chars().take(usr_inpt.len() - 1).collect();

    match temperature.parse() {
        Ok(num) => num,
        Err(_) => 0,
    };
}

Then we tie it all together in main.

fn main() {
    println!("Enter Temperature as 56f or 98c");

    let mut usr_inpt = String::new();

    io::stdin()
        .read_line(&mut usr_inpt)
        .expect("can not read user input");

    let temperature = parse_input(usr_inpt.trim_end().to_lowercase());

    match temperature {
        Temperature::Celsius { celsius } => println!("celcius -> {}", celcius.to_fahrenheit()),
        Temperature::Fahrenheit { fahrenheit } => println!(“fahrenheit -> {}”, fahrenheit.to_celsius()),
        Err(reason) => eprintln!(reason)
    }
}

While it’s certainly more code, it raises the level of abstraction in your main function quite a bit.

  • Greet user
  • Get input
  • Parse input
  • Write out results

This also provides the opportunity (left as an exercise for the reader) to propagate errors from parse_num all the way back up to the user, rather than silently returning an invalid result.

\$\endgroup\$
  • \$\begingroup\$ This is helpful, So not completely but i have updated the code a bit since i am not very comfortable with enums and all in Rust i will follow your advice once i am comfortable with these things.. \$\endgroup\$ – MAK Jan 6 '19 at 7:44
  • \$\begingroup\$ No worries @MAK. I kind of figured. That’s why I broke my answer into two parts. \$\endgroup\$ – RubberDuck Jan 6 '19 at 13:38

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.