1
\$\begingroup\$

Problem statement:

Write a program that converts all given temperatures from a given input temperature scale to a given output temperature scale. The temperature scales to be supported are Kelvin, Celsius, Fahrenheit, Rankine, Delisle, Newton, Rømer, Réaumur.

Synopsis: tempconv INPUT_SCALE OUTPUT_SCALE [TEMPERATURE]...

The INPUT_SCALE and OUTPUT_SCALE shall be given as follows:

  • K for Kelvin
  • C for Celsius
  • F for Fahrenheit
  • R for Rankine
  • D for Delisle
  • N for Newton
  • for Rømer
  • for Réaumur.

Example:

tempconv K C 0 273.15 373.15
-273.15
0.0
100.0

My solution in Rust:

use std::env;

fn main() {
    struct TemperatureConverter {
        to_kelvin: fn(other: f64) -> f64,
        from_kelvin: fn(kelvin: f64) -> f64,
        names: Vec<String>,
    }

    let converters = &[
        TemperatureConverter{to_kelvin: |celsius|    celsius + 273.15,                     from_kelvin: |kelvin| kelvin - 273.15,                       names: vec!["°C".to_string(), "C".to_string(), "c".to_string()]},
        TemperatureConverter{to_kelvin: |kelvin|     kelvin,                               from_kelvin: |kelvin| kelvin,                                names: vec!["K".to_string(), "k".to_string()]},
        TemperatureConverter{to_kelvin: |fahrenheit| (fahrenheit + 459.67) * 5.0 / 9.0,    from_kelvin: |kelvin| kelvin * 9.0 / 5.0 - 459.67,           names: vec!["°F".to_string(), "F".to_string(), "f".to_string()]},
        TemperatureConverter{to_kelvin: |rankine|    rankine * 5.0 / 9.0,                  from_kelvin: |kelvin| kelvin * 9.0 / 5.0,                    names: vec!["°R".to_string(), "R".to_string(), "r".to_string()]},
        TemperatureConverter{to_kelvin: |delisle|    373.15 - delisle * 2.0 / 3.0,         from_kelvin: |kelvin| (373.15 - kelvin) * 3.0 / 2.0,         names: vec!["°De".to_string(), "De".to_string(), "DE".to_string(), "de".to_string()]},
        TemperatureConverter{to_kelvin: |newton|     newton * 100.0 / 33.0 + 273.15,       from_kelvin: |kelvin| (kelvin - 273.15) * 33.0 / 100.0,      names: vec!["°N".to_string(), "N".to_string(), "n".to_string()]},
        TemperatureConverter{to_kelvin: |réaumur|    réaumur * 5.0 / 4.0 + 273.15,         from_kelvin: |kelvin| (kelvin - 273.15) * 4.0 / 5.0,         names: vec!["°Ré".to_string(), "°Re".to_string(), "Ré".to_string(), "RÉ".to_string(), "ré".to_string(), "Re".to_string(), "RE".to_string(), "re".to_string()]},
        TemperatureConverter{to_kelvin: |rømer|      (rømer - 7.5) * 40.0 / 21.0 + 273.15, from_kelvin: |kelvin| (kelvin - 273.15) * 21.0 / 40.0 + 7.5, names: vec!["°Rø".to_string(), "°Ro".to_string(), "Rø".to_string(), "RØ".to_string(), "rø".to_string(), "Ro".to_string(), "RO".to_string(), "ro".to_string()]},
    ];
    let get_converter = |name: &String| {
        for converter in converters {
            if converter.names.contains(name) {
                return converter
            }
        }
        std::process::exit(1)
    };
    let args: Vec<String> = env::args().collect();
    let to_kelvin = get_converter(&args[1]).to_kelvin;
    let from_kelvin = get_converter(&args[2]).from_kelvin;
    for n in &args[3..] {
        println!("{}", from_kelvin(to_kelvin(n.parse::<f64>().unwrap())));
    }
}

I'm especially interested in feedback about how to make the converter definitions shorter. I'm particularly unhappy about the named parameters, is there a way to initialze structs with positional parameters instead? And I'm even more unhappy about the string handling in the names vector. There must be a smarter way, but I'm new to Rust.

\$\endgroup\$

1 Answer 1

9
\$\begingroup\$

This looks like a pretty good first effort, but there are definitely some stylistic issues here as well as a few more advanced features that would make this a lot better. Getting started with the stylistic issues:

Use rustfmt

Rust ships with a formatter rather than a style guide, and you should use it. It helps keep your code clear to others and consistent with other rust code. Invoke it either with cargo fmt or rustfmt [filename] depending on whether you are using cargo or not.

Prefer defining structs outside of functions

Not really a huge issue if very little behavior is defined, but it helps keep your functions decluttered and lets other parts of the code reuse abstractions

Avoid non-Ascii characters in identifiers where possible

They can be tricky to type and are prone to issues like the fact that é is one character and is two different ones. Rust also does not perform any sort of normalization, which compounds the problem. Use an ascii-fied version (like romer) instead.

Prefer &str to &String

See this StackOverflow question for details.

Avoid panicking in application code

If application code panics that should mean something has gone wrong and it's the application's fault, not the users. If there is a user error you should either eprintln a message to standard error then abort, or preferably return an error from main. The instances to watch out for are the indexing into the Vec of args and float parsing.

Every type should implement Debug

When something goes wrong, this will save you a lot of pain because 95% of debugging in rust is just throwing in a couple of dbg! calls and seeing what happens. In most cases, the built-in derive macro can do the job, but you can implement it manually if needed.

#[derive(Debug)]
struct TemperatureConverter {
    to_kelvin: fn(other: f64) -> f64,
    from_kelvin: fn(kelvin: f64) -> f64,
    names: Vec<String>,
}

Avoid collecting the Args iterator

Unless you need to refer back to the unparsed args don't bother collecting, just use Iterator::next;

let mut args = env::args().skip(1);
let from = args.next().unwrap_or_else(|| /* handle error */ );
let to = args.next().unwrap_or_else(|| /* handle error */ );
// and so on

That's it for the more stylistic stuff, but now for the more structural stuff.

Use an enum

There are other ways you could go, but in my opinion, the best way to model the problem is with an enum.

#[derive(Debug)] //add more derives as needed
enum TempScale{
    Celsius,
    Kelvin,
    Fahrenheit,
    Rankine,
    Delisle,
    Newton,
    Reaumur,
    Romer
}

Then add to/from_kelvin methods:

impl TempScale {
    fn to_kelvin(&self, temp:f64) -> f64{
        match self{
            TempScale::Celsius => temp + 273.15,
            TempScale::Kelvin => temp,
            TempScale::Fahrenheit => (temp + 459.67) * 5.0 / 9.0,
            TempScale::Rankine => temp * 5.0 / 9.0,
            TempScale::Delisle => 373.15 - temp * 2.0 / 3.0,
            TempScale::Newton =>  temp * 100.0 / 33.0 + 273.15,
            TempScale::Reaumur => temp * 5.0 / 4.0 + 273.15,
            TempScale::Romer => (temp - 7.5) * 40.0 / 21.0 + 273.15,
        }
    }
    fn from_kelvin(&self, k:f64) -> f64{
        match self{
            TempScale::Celsius => k - 273.15,
            TempScale::Kelvin => k,
            TempScale::Fahrenheit => k  * 9.0 / 5.0 - 459.67,
            TempScale::Rankine => k  * 9.0 / 5.0,
            TempScale::Delisle => (373.15 - k) * 3.0 / 2.0,
            TempScale::Newton => (k - 273.15) * 33.0 / 100.0,
            TempScale::Reaumur => (k - 273.15) * 4.0 / 5.0,
            TempScale::Romer => (k - 273.15) * 21.0 / 40.0 + 7.5,
        }
    }
}

Then, add a proper FromStr implementation, taking into account that rust does have a Unicode aware to_lowercase function.

use std::str::FromStr;

impl FromStr for TempScale {
    type Err = String; //FIXME: this should be a dedicated error type, rather than a string
    fn from_str(mut s: &str) -> Result<Self, Self::Err> {
        s = s.trim().trim_start_matches('°');
        match s.to_lowercase().as_str() {
            "c" => Ok(TempScale::Celsius),
            "k" => Ok(TempScale::Kelvin),
            "f" => Ok(TempScale::Fahrenheit),
            "r" => Ok(TempScale::Rankine),
            "de" => Ok(TempScale::Delisle),
            "n" => Ok(TempScale::Newton),
            "ré" => Ok(TempScale::Reaumur),
            "re" => Ok(TempScale::Reaumur),
            "ro" => Ok(TempScale::Romer),
            "rø" => Ok(TempScale::Romer),
            other => Err(format!("`{other}` is not a temperature scale")),
        }
    }
}

Note that this is deliberately more permissive than what you currently have, as allows excess whitespace and allows every scale to be preceded with a degree symbol. This may or may not be acceptable depending on your application.

All that code reduces main to just

fn main() {
    let mut args = env::args().skip(1);
    let from = args
        .next()
        .expect("handle this error properly")
        .parse::<TempScale>()
        .expect("handle this error properly");
    let to = args
        .next()
        .expect("handle this error properly")
        .parse::<TempScale>()
        .expect("handle this error properly");
    for num in args {
        let temp = num.parse::<f64>().expect("handle this error properly");
        println!("{}", to.from_kelvin(from.to_kelvin(temp)));
    }
}

which is a lot nicer than it was previously. I've left proper error handling as an exercise for the reader, mainly because kinda boilerplatey, and the best way to do it depends on whether or not external libraries are allowed. Playground for the example can be found here.

I have a few other optional notes, depending on your use case.

(Optional) Don't println! in a loop

While using println is quick, easy, and good enough for most use cases, it can often be a surprising performance bottleneck. Internally, println acquires and releases a lock on stdout every time it's called. This doesn't matter most of the time, but if you find yourself converting a few hundred million different temperatures per invocation, you should probably look up std::io and pick a more appropriate output strategy.

(Optional) Use a proper CLI parser

There are a whole bunch of proper CLI arg parsers out there, and it's almost always better to use one of them rather than roll your own parsing. Clap is the only one I have any experience with but it handles things like subcommands, flags, aliases, etc. pretty much painlessly, as well as giving nicely formatted help messages for free. There are plenty of other options out there, like pico-args if you want something more lightweight.

(Optional) Use external libraries to ease error handling

I've kinda deliberately avoided writing proper error handling, because it's kinda boilerplatey by default, especially if you are converting from Options to Results. However, there are a couple of crates that can make this a lot easier.

Use thiserror to easily create custom error types. Thiserror lets you derive the Error trait as well as a couple of other traits necessary for conveniently handling errors with proc macros. With thiserror, the FromStr implementation looks like this:

use std::str::FromStr;
use thiserror::Error;
#[derive(Debug, Error)]
#[error("`{}` is not a temperature scale", self.0)]
pub struct TempParseError(String);


impl FromStr for TempScale {
    type Err = TempParseError; 
    fn from_str(mut s: &str) -> Result<Self, Self::Err> {
        s = s.trim().trim_start_matches('°');
        match s.to_lowercase().as_str() {
            "c" => Ok(TempScale::Celsius),
            "k" => Ok(TempScale::Kelvin),
            "f" => Ok(TempScale::Fahrenheit),
            "r" => Ok(TempScale::Rankine),
            "de" => Ok(TempScale::Delisle),
            "n" => Ok(TempScale::Newton),
            "ré" => Ok(TempScale::Reaumur),
            "re" => Ok(TempScale::Reaumur),
            "ro" => Ok(TempScale::Romer),
            "rø" => Ok(TempScale::Romer),
            other => Err(TempParseError(other.into())),
        }
    }
}

which is basically the same, but now it integrates with the rust ecosystem much better.

Use anyhow for effortless error propagation. anyhow::Error works like Box<dyn Error> but better, and anyhow allows easy conversion of Options to Results. The main function I had above can be rewritten with anyhow as

fn main() -> anyhow::Result<()> {
    let mut args = env::args().skip(1);
    let from = args
        .next()
        .context("too few arguments")?
        .parse::<TempScale>()?;
    let to = args
        .next()
        .context("too few arguments")?
        .parse::<TempScale>()?;
    for num in args {
        let temp = num.parse::<f64>()?;
        println!("{}", to.from_kelvin(from.to_kelvin(temp)));
    }
    Ok(())
}

which is much easier and provides decent error messages out of the box. Playground, now with proper error handling.

(Optional) Handle Unicode smarter

Unicode sucks to work with, and a big part of that is because there is more than one way to encode many glyphs, as I mentioned earlier. Pulling in an external crate to help deal with this might be a good idea, depending on your use case. I don't know the best way to deal with this, but unicode-normalization is probably a good place to start.

(Optional) Don't be afraid of dependencies

Rust's standard library is pretty barebones for a reason. Cargo makes managing dependencies painless, especially when compared to almost every other programming language. Most of the time, adding a dependency just requires a single line in your Cargo.toml and everything just works. Adding dependencies isn't always an option, but when it is, don't be afraid to do it.

(Optional) Write some tests

Support for tests is built into rustc. It's pretty easy to write some small tests most of the time, something like

#[test]
fn convert_celius_to_kelvin(){
    assert_eq!(TempScale::Celsius.to_kelvin(0), 273.15);
}

but obviously, you probably want more coverage than that. I say tests are optional in this case, mainly because floating point equality is annoyingly hard, although there are crates to help with that, and the problem specification feels like there's an external tool for testing somewhere. Realistically, if you can have tests, you probably should.

That about wraps up what I have to say. Since you seem to come from a Java background, I will give you a word of warning- while some OOP patterns translate well to Rust, a lot of them don't. In particular, Rust favors composition over inheritance, and if you find yourself reaching for Rc<RefCell<T>>, it's probably time to reevaluate your approach. Anyway, welcome to rust, I hope you have a good time.

\$\endgroup\$
0

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.