5
\$\begingroup\$

Below is a simple Rust program which inter-converts temperature between the Celsius scale and the Fahrenheit scale. It takes two inputs:

  1. scale referring to the temperature scale of the next input
  2. the value you you want to convert

use std::io;

fn main() {
    loop {

        println!("Enter the temperature scale ('c' for Clesius and 'f' for Fahrenheit):");
        let mut choice: String = String::new();
        io::stdin()
            .read_line(&mut choice)
            .expect("Could not read the input!");

        let scale: char = match choice.trim() { // `\n` is also included in choice
            "c" => 'c',
            "C" => 'c',
            "f" => 'f',
            "F" => 'f',
            _ => {
                println!("Invalid scale chosen! Please try again.");
                continue
            }
        };

        println!("Enter the temperature:");
        let mut input: String = String::new();
        io::stdin()
            .read_line(&mut input)
            .expect("Could not read the input!");

        let temp: f64 = match input.trim().parse() {
            Ok(num) => num,
            Err(_) => {
                println!("Please enter a number!");
                continue
            }
        };

        let result = match scale {
            'c' => c_to_f(temp),
            'f' => f_to_c(temp),
            _ => 0.0
        };

        println!("The converted temperature is: {result}");
    };
}

fn f_to_c (f: f64) -> f64 {
    (f - 32.0) * 5.0 / 9.0
}

fn c_to_f (c: f64) -> f64 {
    c * 9.0 / 5.0 + 32.0
}

I starting learning Rust recently and wanted to know if the style that I have written in is good and if there are any code quality issues with my code.

Are there any good standards that I can read from or books/online resources where I can learn a few basic rules?

\$\endgroup\$
7
  • 2
    \$\begingroup\$ The code looks good to me. The only things I would change are the extra blank lines, and adding a semicolon after the continues. In more performance critical examples, I would also declare the String outside the loop to not reallocate each time. \$\endgroup\$ Aug 22, 2023 at 11:21
  • \$\begingroup\$ Cannot help with rust but fyi, typo: "Clesius". \$\endgroup\$
    – Marvin
    Aug 22, 2023 at 11:37
  • \$\begingroup\$ @Marvin Thanks for pointing out! \$\endgroup\$ Aug 23, 2023 at 10:58
  • \$\begingroup\$ @ChayimRefaelFriedman Can't it be expected from a current compiler to do that kind of optimization? I think for the purpose of readability the code as is is preferable to an optimized version as the string is reassigned every loop iteration, right? \$\endgroup\$
    – U. Windl
    Aug 23, 2023 at 12:23
  • \$\begingroup\$ Actually I don't program in Rust, but, given the simple nature of the two converter functions, wouldn't it make sense to store them as closures in a hash indexed by the commands, eliminating the if (given a valid hash key was there)? You could also map all invalid input to one error value, adding a third generic "error closure"... \$\endgroup\$
    – U. Windl
    Aug 23, 2023 at 12:28

3 Answers 3

7
\$\begingroup\$

Use cargo fmt

or rustfmt to format your code. Then use clippy to lint it. Many IDEs allow cargo fmt-ing the code when saving a file.

Know your functions

&str does not only have a trim() method, but also to_ascii_lowercase():

const ALLOWED_UNIS: [&str; 2] = ["c", "f"];

...


        
        if !ALLOWED_UNIS.contains(&choice.trim().to_ascii_lowercase().as_str()) {
            println!("Invalid scale chosen! Please try again.");
            continue;
        }
...

Represent units as enum variants

You can use an enum to represent the two temperature units. Since you're currently only using two, the conversion may be trivially implemented since we can default the target to the respective other unit:

enum Temperature {
    Fahrenheit(f64),
    Celsius(f64),
}

impl Temperature {
    pub fn convert(&self) -> Self {
        match self {
            Self::Fahrenheit(fahrenheit) => Self::Celsius((fahrenheit - 32.0) * 5.0 / 9.0),
            Self::Celsius(celsius) => Self::Fahrenheit(celsius * 9.0 / 5.0 + 32.0),
        }
    }
}

Implement useful traits

Since you parse the unit and the value seperately, you can implement TryFrom for a value / unit tuple to create a temperature variant:



impl TryFrom<(f64, &str)> for Temperature {
    type Error = &'static str;

    fn try_from((value, unit): (f64, &str)) -> Result<Self, Self::Error> {
        match unit.trim().to_ascii_lowercase().as_str() {
            "c" => Ok(Self::Celsius(value)),
            "f" => Ok(Self::Fahrenheit(value)),
            _ => Err("unsupported unit"),
        }
    }
}

We can also implement TryFrom to parse the value from a string beforehand and then delegate to the other implementation above:

impl TryFrom<(&str, &str)> for Temperature {
    type Error = &'static str;

    fn try_from((value, unit): (&str, &str)) -> Result<Self, Self::Error> {
        Self::try_from((
            value.trim().parse::<f64>().map_err(|_| "not a number")?,
            unit,
        ))
    }
}

And in order to properly display those, you can implement the std::fmt::Display trait:



impl Display for Temperature {
    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
        match self {
            Self::Celsius(value) => write!(f, "{} °C", value),
            Self::Fahrenheit(value) => write!(f, "{} °F", value),
        }
    }
}

Putting it together

We have implemented the different temperature units as enum variants and allowed conversion between the two of them. We also have outsourced some of the parsing into a TryFrom trait implementation. Further we have implemented the Display trait. Thus we have splitted up input, processing and output into separate components, though the input processing and parsing can still be improved upon:

use std::fmt::{Display, Formatter};
use std::io;

enum Temperature {
    Celsius(f64),
    Fahrenheit(f64),
}

impl Temperature {
    pub fn convert(&self) -> Self {
        match self {
            Self::Celsius(celsius) => Self::Fahrenheit(celsius * 9.0 / 5.0 + 32.0),
            Self::Fahrenheit(fahrenheit) => Self::Celsius((fahrenheit - 32.0) * 5.0 / 9.0),
        }
    }
}

impl Display for Temperature {
    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
        match self {
            Self::Celsius(value) => write!(f, "{value} °C"),
            Self::Fahrenheit(value) => write!(f, "{value} °F"),
        }
    }
}

impl TryFrom<(f64, &str)> for Temperature {
    type Error = &'static str;

    fn try_from((value, unit): (f64, &str)) -> Result<Self, Self::Error> {
        match unit.trim().to_ascii_lowercase().as_str() {
            "c" => Ok(Self::Celsius(value)),
            "f" => Ok(Self::Fahrenheit(value)),
            _ => Err("unsupported unit"),
        }
    }
}

impl TryFrom<(&str, &str)> for Temperature {
    type Error = &'static str;

    fn try_from((value, unit): (&str, &str)) -> Result<Self, Self::Error> {
        Self::try_from((
            value.trim().parse::<f64>().map_err(|_| "not a number")?,
            unit,
        ))
    }
}

fn main() {
    loop {
        println!("Enter the temperature scale ('c' for Celsius and 'f' for Fahrenheit):");
        let mut unit = String::new();
        io::stdin()
            .read_line(&mut unit)
            .expect("Could not read the input!");

        println!("Enter the temperature:");
        let mut value = String::new();
        io::stdin()
            .read_line(&mut value)
            .expect("Could not read the input!");

        match Temperature::try_from((value.as_str(), unit.as_str())) {
            Ok(temperature) => {
                println!("The converted temperature is: {}", temperature.convert());
            }
            Err(message) => eprintln!("{message}"),
        }
    }
}

And with Chayim's hint included:

use std::fmt::{Display, Formatter};
use std::io;

enum Temperature {
    Celsius(f64),
    Fahrenheit(f64),
}

impl Temperature {
    pub fn convert(&self) -> Self {
        match self {
            Self::Celsius(celsius) => Self::Fahrenheit(celsius * 9.0 / 5.0 + 32.0),
            Self::Fahrenheit(fahrenheit) => Self::Celsius((fahrenheit - 32.0) * 5.0 / 9.0),
        }
    }
}

impl Display for Temperature {
    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
        match self {
            Self::Celsius(value) => write!(f, "{value} °C"),
            Self::Fahrenheit(value) => write!(f, "{value} °F"),
        }
    }
}

impl TryFrom<(f64, &str)> for Temperature {
    type Error = &'static str;

    fn try_from((value, unit): (f64, &str)) -> Result<Self, Self::Error> {
        match unit.trim().to_ascii_lowercase().as_str() {
            "c" => Ok(Self::Celsius(value)),
            "f" => Ok(Self::Fahrenheit(value)),
            _ => Err("unsupported unit"),
        }
    }
}

impl TryFrom<(&str, &str)> for Temperature {
    type Error = &'static str;

    fn try_from((value, unit): (&str, &str)) -> Result<Self, Self::Error> {
        Self::try_from((
            value.trim().parse::<f64>().map_err(|_| "not a number")?,
            unit,
        ))
    }
}

fn main() {
    let mut unit = String::new();
    let mut value = String::new();

    loop {
        println!("Enter the temperature scale ('c' for Celsius and 'f' for Fahrenheit):");
        unit.clear();
        io::stdin()
            .read_line(&mut unit)
            .expect("Could not read the input!");

        println!("Enter the temperature:");
        value.clear();
        io::stdin()
            .read_line(&mut value)
            .expect("Could not read the input!");

        match Temperature::try_from((value.as_str(), unit.as_str())) {
            Ok(temperature) => {
                println!("The converted temperature is: {}", temperature.convert());
            }
            Err(message) => eprintln!("{message}"),
        }
    }
}
\$\endgroup\$
3
  • \$\begingroup\$ I found the enums the most useful, thank you. \$\endgroup\$ Aug 23, 2023 at 11:01
  • \$\begingroup\$ For someone coming from an OO language, those pattern matches on the type of self look "wrong", since one of the fundamental rules of OO is that self knows its own type at all times, so it never needs to look it up, and invoking different behavior based on self's type is just plain old message dispatch, which is built into OO. That's an interesting pattern to get used to, I imagine. \$\endgroup\$ Aug 23, 2023 at 14:59
  • \$\begingroup\$ Some Rust concepts need some getting used to, indeed. I myself come from Python and Java. The fact that Rust does not support inheritance e.g., baffled me at first. But its other language concepts, especially the tagged unions aka. enums and trait system more than make up for it, IMHO. \$\endgroup\$ Aug 23, 2023 at 17:32
5
\$\begingroup\$

“Idiomatic” is in the Eye of the Beholder—But!

I’m also, for the purposes of this example, going to tweak the program so that it reads multiple lines of input, accepting any of the forms:

  • -0.0 F, 0 C, 0 f, 0 c
  • 0F, 0C, 0f, 0c
  • 0°F, 0°C (where ° is U+00B0)
  • 0℃ (where is U+2103)
  • 0℉ (where is U+2109)

Rust supports many different programming styles, and there will often be a piece of code that’s a lot more elegant to express in an unusual way. Still: Some things are a lot more idiomatic than others.

Let’s start with:

Static Single Assignments

Rust made these the default, and makes you write mut if you want anything else. Much of its syntax—including match, irrefutable if let patterns, the way for works, while let to re-initialize a loop variable on every iteration, and blocks as expressions—is there to allow you express the operations without needing mutable state.

Not only does this eliminate entire classes of logic errors that often crop up in early C, Rustc is built on top of LLVM, which converts all code to SSA form anyway (and the other experimental compiler projects also use backends whose intermediate representation is in SSA form). So code written this way will usually compile quickly and optimize well.

Therefore, write an algorithm in Rust without mut if you can. There are a couple of these in the code, so let’s refactor them:

let mut choice: String = String::new();
// ...
let mut input: String = String::new();

(Side note: you can leave out type annotations like : String when both the compiler and the human maintainer can easily infer them. There’s no need to say String twice on the same line.)

What’s a good pattern to replace these with?

Iterator Expressions

Iterators are a big feature of Rust. They’re designed to be used in fluent style, with either closures or helper functions. Earlier, I said I was going to change the program to iterate over each line of input, and iterators are a great way to do that. It’s such a common idiom that there’s a function in the standard library to turn a handle to standard input into an iterator over the lines of input:

use std::io::stdin;

pub fn main() {
    for line in stdin().lines() {

This actually returns a Result that is either a valid input line or an error. To keep things simple, let’s panic on any read error, for now.

    let lines = stdin().lines().map(Result::unwrap); // Error recovery not implemented.

    for line in lines {

What’s the type of lines? Some kind of iterator over String, so line is a String. The compiler will figure it out.

Data Types

Here, there are two pieces of information we want to extract from each line, a scalar, and which scale it’s in. You chose to represent the temperature as a f64. That’s a great idea, and I’ll leave that part of your code alone.

There are two possible choices of unit, which naturally lends itself to an enum:

enum Scale {
    Celsius,
    Fahrenheit,
}

The Loop Body

Above, we started with a for loop that iterates over each line of input, as a String. The “only” thing left is to fill in the body of this loop.

Although it would be possible to borrow the input line and extract each piece of information separately with a different call, it makes more sense here to parse the temperature into a tuple, with a scalar and a scale constant. Since the string could be invalid, parsing it could fail. Therefore, the result of a parse should be an Option type. Finally, stringy inputs should normally be passed as &str if we aren’t going to consume or alter them. This means the parse function will have the signature:

fn parse_temperature(line: &str) -> Option<(f64, Scale)>

(The compiler will actually enforce that function names should be snake_case, typenames should be PascalCase and constants should be UPPERCASE.)

We can fill in the body with {todo!()} for now, but we’ll come back to this later.

Exhaustive match Expressions

Once we’ve implemented the parse_temperature function, main can look something like this:

pub fn main() {
    println!("Enter a temperature, such as -32 F or 0.0°C.");

    let lines = stdin().lines().map(Result::unwrap); // Error recovery not implemented.

    for line in lines {
        match parse_temperature(&line) {
            Some((t, Scale::Celsius)) => {todo!()},
            Some((t, Scale::Fahrenheit)) => {todo!()},
            None => {todo!()},
        }
    }
}

Not only is this simple, it’s robust against changes. If we later add a Kelvin scale, the compiler will notice that our match patterns no longer cover every case exhaustively, and tell us that we need to handle Scale::Kelvin.

Helper Functions

Rust makes heavy use of these, both on the right-hand side of static single assignments (what computer scientists call “phi functions,” because “phony functions” sounded too informal) and to pass to higher-order functions like map. It’ll often be shorter and easier to write a shorthand like an if block, match statement or closure than a named function.

However you do it, composing your algorithms from small pieces that let you use patterns such as map, fold and railway-oriented programming is idiomatic Rust, and a large part of the standard library is written to enable this.

Let’s now come back to parse_temperature:

fn parse_temperature(line: &str) -> Option<(f64, Scale)>

What pieces does the standard library give us to turn an input string into a scalar and an enum representing its unit? Well, we have str::parse(), which parses a string into an arbitrary type. If we can extract from the original string the slices containing the scalar and the unit label, we can pass the first slice to str::<f64>::parse. And if we impl FromStr for Scale (which we’ll come back to later), we will then be able to pass the second slice to str::<Scale>::parse as well. So let’s write another helper, and call it tokenize.

Writing a pattern for parse_temperature that’s irrefutable (producing a value on every possible branch) would end up looking like a pyramid of doom, though:

    if let Some((scalar_str, unit_str)) = tokenize(line) {
        if let Ok(t) = scalar_str.parse() {
            if let Ok(u) = unit_str.trim_start().parse() {
                Some((t, u))
            } else {
                None
            }
        } else {
            None
        }
    } else {
        None
    }

So you might (or might not) prefer a version without else. Because we refactored this into a helper function, we can use return to write one:

    if let Some((scalar_str, unit_str)) = tokenize(line) {
        if let Ok(t) = scalar_str.parse() {
            if let Ok(u) = unit_str.trim_start().parse() {
                return Some((t, u));
            }
        }
    }

    None
}

Note that we don’t need to use any “turbofish operators” on this code: because one possible return value is Some((t,u)), the compiler can infer the types of t and u from the return type of the function, and apply that to the if let statements.

We now need to implement tokenize. If we look through the documentation of the str type, we see two member functions that can do it: str::find locates the byte index of the first character in a string that matches a predicate, and str::split_at splits a string into two slices: the substring up to an index, and the substring starting at the index. What predicate would make that work? A predicate that returns true for the first character that is not part of a valid f64. Decomposed that way, both pieces become simple:

    const fn not_fp(c: char) -> bool {
        match c {
            '+' | '-' | '.' | 'e' | '0' | '1' | '2' | '3' | '4' | '5' | '6' | '7' | '8' | '9' => {
                false
            }
            _ => true,
        }
    }

    fn tokenize(input: &str) -> Option<(&str, &str)> {
        input.find(not_fp).map(move |i| input.split_at(i))
    }

The implementation of tokenize uses a bit of railway-oriented style: if the find call returns Some(index), map will pass index to the closure and wrap its result in Some. If, on the other hand, find returns None, the algorithm will short-circuit and return None.

Also note that, even though we used borrows, we didn’t need any lifetime annotations. In some other case, we might. Here, since we borrowed our input and returned two borrowed slices as our outputs, the compiler assumed by default that the lifetimes were all the same. Which is correct: the tokens are substrings of the input, with the same lifetime.

The not_fp function is a match expression like others we’ve seen. One quirk of it is that it’s the only function in the program that can be declared const. Every other one calls at least one trait function, which cannot be const; or does floating-point math, which cannot be const in LLVM; or calls several different non-const functions. So, it’s a good idea to declare your functions const fn when you can, but here and in most other situations, it’s not actually useful.

Implement Useful Traits

As mentioned, we’ll need to implement the FromStr trait for parsing to a Scale to work. One possible implementation is:

impl FromStr for Scale {
    type Err = ();
    fn from_str(s: &str) -> Result<Self, <Self as FromStr>::Err> {
        match s {
            "C" | "c" | "\u{00B0}C" | "\u{2103}" => Ok(Scale::Celsius),
            "F" | "f" | "\u{00B0}F" | "\u{2109}" => Ok(Scale::Fahrenheit),
            _ => Err(()),
        }
    }
}

It’s important to remember that Rust strings are not arrays of char, but contain UTF-8 byte data. Fortunately, we can pattern-match them.

The most interesting decision here is that the trait allows us to return an arbitrary Err on failure. But this program doesn’t actually need to know anything about the error, beyond the fact that one occurred, so we can just make our Err type the empty type, ().

Although we don’t actually use == or .clone() on Scale values in this program, it’s good practice to derive trait implementations that make sense for your type, in this case

#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]

Putting it All Together

use std::io::stdin;
use std::str::FromStr;

#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
enum Scale {
    Celsius,
    Fahrenheit,
}

impl FromStr for Scale {
    type Err = ();
    fn from_str(s: &str) -> Result<Self, <Self as FromStr>::Err> {
        match s {
            "C" | "c" | "\u{00B0}C" | "\u{2103}" => Ok(Scale::Celsius),
            "F" | "f" | "\u{00B0}F" | "\u{2109}" => Ok(Scale::Fahrenheit),
            _ => Err(()),
        }
    }
}

fn parse_temperature(line: &str) -> Option<(f64, Scale)> {
    const fn not_fp(c: char) -> bool {
        match c {
            '+' | '-' | '.' | 'e' | '0' | '1' | '2' | '3' | '4' | '5' | '6' | '7' | '8' | '9' => {
                false
            }
            _ => true,
        }
    }

    fn tokenize(input: &str) -> Option<(&str, &str)> {
        input.find(not_fp).map(move |i| input.split_at(i))
    }

    if let Some((scalar_str, unit_str)) = tokenize(line) {
        if let Ok(t) = scalar_str.parse() {
            if let Ok(u) = unit_str.trim_start().parse() {
                return Some((t, u));
            }
        }
    }

    None
}

fn f_to_c(f: f64) -> f64 {
    (f - 32.0) * 5.0 / 9.0
}

fn c_to_f(c: f64) -> f64 {
    c * 9.0 / 5.0 + 32.0
}

pub fn main() {
    println!("Enter a temperature, such as -32 F or 0.0°C.");

    let lines = stdin().lines().map(Result::unwrap); // Error recovery not implemented.

    for line in lines {
        match parse_temperature(&line) {
            Some((t, Scale::Celsius)) => {
                println!("{}°C is {}°F.", t, c_to_f(t));
            }
            Some((t, Scale::Fahrenheit)) => {
                println!("{}°F is {}°C.", t, f_to_c(t));
            }
            None => {
                println!("Parse error. Re-enter a temperature, such as -32 F or 0.0°C.");
            }
        }
    }
}

And a link to the Godbolt compiler explorer, with testcases.

\$\endgroup\$
1
  • \$\begingroup\$ Will look into traits, thanks! \$\endgroup\$ Aug 23, 2023 at 11:06
4
\$\begingroup\$

Remove the type annotations

You have annotated all of your local variables with their types (ex. let mut input: String = ...). Usually in Rust you let the compiler infer the type of local variables, unless adding in the type helps with readability or is necessary because the compiler is unable to infer its type.

Use an enum for the scale

Instead of storing the scale as a character that is either 'f' or 'c', create a enum for distinguishing the scales; for example:

enum Unit {
    Fahrenheit,
    Celsius,
}

This lets the compiler know that there are only two possible values, and checks that you handle them properly. This will let you remove the unused _ => 0.0 branch in the conversion match.

Use or-patterns in the unit match

This:

        let scale: char = match choice.trim() { // `\n` is also included in choice
            "c" => 'c',
            "C" => 'c',
            "f" => 'f',
            "F" => 'f',
            _ => {
                println!("Invalid scale chosen! Please try again.");
                continue
            }
        };

Can be shortened to:

        let scale: char = match choice.trim() { // `\n` is also included in choice
            "c" | "C" => 'c',
            "f" | "F" => 'f',
            _ => {
                println!("Invalid scale chosen! Please try again.");
                continue
            }
        };

Since inputting "c" or "f" in either case should not affect the chosen units. Strings also have some case related methods.

Re-using the input buffer

If you create a single String variable before the loop, and use it with both read_line calls while clearing it beforehand, the program will re-use the same buffer while reading, instead of allocating a new buffer each time like you are doing now. This is unlikely to affect the performance of your interactive app, but can make a difference in other situations that involve a lot of reading.

\$\endgroup\$
1
  • \$\begingroup\$ TIL the | - or-patterns, thanks. \$\endgroup\$ Aug 23, 2023 at 11:02

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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