I hope some late help will still be helpful!
I've erred on the side of providing a bit too much feedback, but this may give you a sense of some of the styles and conventions that Rust uses even if they are overkill for this simple program.
Nicely organized code overall, very understandable and clear use of functions.
In function arguments, it's generally cleaner/more efficient/more idiomatic to accept &str
instead of String
. This is usually confusing for beginners, but &str
is read-only string data and String
is an owned, dynamically growable string, so basically if you only need read-only, you should use &str
. As a rule of thumb, it's a good starting point to just make all function arguments &str
and the return value String
. Here's output
fixed; noticed that this means you don't have to to .to_string()
in your other functions:
fn c_to_f(c: f32) {
output(c * 1.8 + 32.0, "F");
}
fn f_to_c(f: f32) {
output((f - 32.0) / 1.8, "C");
}
fn output(t: f32, u: &str) {
println!("The output temperature is: {}{}", t, u);
}
Although it doesn't make a huge difference for a small project like this, another good rule of thumb is to avoid functions with printing behavior; instead functions should return Strings, and only the top-level program (i.e. main function) prints. So let's change fn output
again. All we need to do is use format!
instead of println!
(and print in main
):
fn c_to_f(c: f32) -> String {
output(c * 1.8 + 32.0, "F")
}
fn f_to_c(f: f32) -> String {
output((f - 32.0) / 1.8, "C")
}
fn output(t: f32, u: &str) -> String {
format!("The output temperature is: {}{}", t, u)
}
The let value: f32
logic is a bit verbose, the usual function for this is unwrap_or_else
:
let value: f32 = value.trim().parse().unwrap_or_else(|_| {
panic!("{} is not a number", value)
});
In the line let (value, unit) = temperature.split_at(temperature.len() - 2)
, this use of split_at
is considered fragile and mostly discouraged. Remember that in Rust strings are not just sequences of bytes, but valid UTF-8 data where the characters may not correspond to bytes. So this code introduces a silent failure (i.e. one that is not explicitly exposed and dealt with by the application): I tried running your program on input 4 😍
and I got
thread 'main' panicked at 'byte index 5 is not a char boundary; it is inside '😍'
In fact, there is actually a second similar issue in the same line: when run with an empty input line in release mode, you have integer underflow:
thread 'main' panicked at 'byte index 18446744073709551615 is out of bounds of `
`'
These edge cases do not introduce a bigger problem for your case, but in production code such failures could cause an unexpected crash, or worse, a security vulnerability. So the preference in Rust is always to be explicit about handling errors. You can browse the functions available in the documentation https://doc.rust-lang.org/std/primitive.str.html
for some ideas of how to parse strings in general; here the easiest is probably to explicitly check for both errors:
let mid = temperature.len().checked_sub(2).expect("Input string was empty");
if !temperature.is_char_boundary(mid) {
panic!("Input should end in a single-byte char (the unit)");
}
let (value, unit) = temperature.split_at(mid);
If you only need mutability for a temporary amount of time (as with temperature
), it's good practice to declare the variable immutable after that: let temperature = temperature
The last part of main
can be reduced a bit by using match
instead of if
: match unit.as_str() {
Finally, for extensibility it would be great to abstract temperature into an appropriate type; Rust programmers like to leverage static types to make sure they keep for example Celsius and Fahrenheit values separate, without any actual overhead at runtime. Although I won't change your code for this, you would define types like so:
struct Fahrenheit(f64);
struct Celsius(f64);
You could alternatively have a Temperature type if you want a lot of functionality related to temperatures in general:
enum Temperature {
Fahrenheit(f64),
Celsius(f64),
}
Then, you would define parsing functions (parse from &str
), display functions (convert to String
), and so on on those types, rather than including all of this in main
.
Here's the final code incorporating all the suggestions except the last one:
use std::io;
fn main() {
println!("Enter a temperature.");
let mut temperature = String::new();
io::stdin()
.read_line(&mut temperature)
.expect("Failed to read temperature");
let temperature = temperature;
let mid = temperature.len().checked_sub(2).expect("Input string was empty");
if !temperature.is_char_boundary(mid) {
panic!("Input should end in a single-byte char (the unit)");
}
let (value, unit) = temperature.split_at(mid);
let value: f32 = value.trim().parse().unwrap_or_else(|_| {
panic!("{} is not a number", value)
});
let unit = unit.trim().to_lowercase();
println!("The input temperature is: {}", temperature);
match unit.as_str() {
"c" => println!("{}", c_to_f(value)),
"f" => println!("{}", f_to_c(value)),
_ => panic!("Unsupported unit"),
}
}
fn c_to_f(c: f32) -> String {
output(c * 1.8 + 32.0, "F")
}
fn f_to_c(f: f32) -> String {
output((f - 32.0) / 1.8, "C")
}
fn output(t: f32, u: &str) -> String {
format!("The output temperature is: {}{}", t, u)
}