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 collect
ing, 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 Option
s to Result
s. 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 Option
s to Result
s. 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.