I'm in the process of rewriting my programs from Java into Rust. It's from a course I'm taking in college at the moment. I would greatly appreciate any feedback on my Rust code.
As a disclaimer, none of this was written with AI. I've read through numerous chapters of the Rust book multiple times, but I'm still learning and definitely don't consider myself an experienced programmer. What I haven't learned from the Rust book, I've gathered from using the Rust standard library docs.
One thing to please keep in mind is that this code represents only what we were taught so far in the course, so does not include things like functions or proper error handling. I did, however, refactor the Rust code with functions because I wanted to practice borrowing and returning values. I'm still in the early phases of my Java to Rust rewrite and I have dozens of more programs to go.
Having said that, though, I would greatly appreciate and value any feedback anyone is willing to provide, especially as it relates to idiomatic Rust! Thanks so much!
Here is the original Java code:
import java.util.Scanner;
public class Program3_3 {
public static void main(String[] args) {
Scanner input = new Scanner(System.in);
char next_question = 'y';
int count = 0;
int correct_guess = 0;
while (next_question == 'y') {
int number1 = (int)(Math.random() * 10);
int number2 = (int)(Math.random() * 10);
System.out.print("What is " + number1 + " * " + number2 + "? ");
int user_guess = input.nextInt();
if (user_guess == number1 * number2) {
System.out.println("Correct. Nice work!");
correct_guess++;
}
else {
System.out.println("Incorrect. The product is " + number1 * number2);
}
count++;
input.nextLine();
System.out.print("Want more questions y or n ? ");
String user_question = input.nextLine();
char input_char = user_question.charAt(0);
if (input_char == 'y') {
next_question = 'y';
}
else {
next_question = 'n';
}
}
System.out.println(" You scored " + correct_guess + " out of " + count);
input.close();
}
}
Here is the Rust rewrite:
use std::io;
use std::io::Write;
use rand::Rng;
fn main() {
let mut question: bool = true;
let mut number1: i32;
let mut number2: i32;
let mut count: i32 = 0;
let mut correct_guess: i32 = 0;
while question != false {
number1 = rand::thread_rng().gen_range(0..=10);
number2 = rand::thread_rng().gen_range(0..=10);
let user_guess: i32 = get_int_input(&number1, &number2);
if user_guess == number1 * number2 {
println!("Correct. Nice work!");
correct_guess += 1;
}
else {
println!("Incorrect. The product is {}.", number1 * number2);
}
count += 1;
question = get_bool_input("Want to answer more questions? y or n: ");
}
println!("You scored {correct_guess} out of {count} guesses.");
}
fn get_int_input(number1: &i32, number2: &i32) -> i32 {
print!("What is {number1} * {number2}? ");
let _ = io::stdout().flush();
let mut input = String::new();
io::stdin()
.read_line(&mut input)
.expect("Failed to read line.");
let input: i32 = input.trim().parse().expect("Please type an integer!");
input
}
fn get_bool_input(prompt: &str) -> bool {
print!("{prompt}");
let _ = io::stdout().flush();
let mut input = String::new();
io::stdin()
.read_line(&mut input)
.expect("Failed to read line");
let input = input.trim();
if input == "n" {
return false;
}
else {
return true;
}
}
Thanks again ahead of time and I look forward to learning from any feedback!
... in almost any language.
Your coding-style is quite verbose. F.e. number1 and number2 don't have to be mutable.
question could be left out. Replace "while question == false" with a loop and exit it with "
get_bool_input(...) == false".
I really appreciate you taking the time to read my post and reply. All you points are noted. As to one thing though, if I don't declare number1 and number2 and mutable, I get the following error:
error[E0384]: cannot assign twice to immutable variable `number1`
--> src/main.rs:17:9
|
9 | let number1: i32;
| ------- help: consider making this binding mutable: `mut number1`
...
17 | number1 = rand::thread_rng().gen_range(0..=10);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot assign twice to immutable variable
error[E0384]: cannot assign twice to immutable variable `number2`
--> src/main.rs:18:9
|
10 | let number2: i32;
| ------- help: consider making this binding mutable: `mut number2`
...
18 | number2 = rand::thread_rng().gen_range(0..=10);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot assign twice to immutable variable
For more information about this error, try `rustc --explain E0384`.
error: could not compile `program3` (bin "program3") due to 2 previous errors
Perhaps someone could please explain this. I originally had:
fn get_bool_input(prompt: &str) -> bool {
print!("{prompt}");
let _ = io::stdout().flush();
let mut input = String::new();
io::stdin()
.read_line(&mut input)
.expect("Failed to read line");
let input = input.trim();
if input == "n" {
return false;
}
else {
return true;
}
And it was suggested I refactor this to:
n get_bool_input(prompt: &str) -> bool {
print!("{prompt}");
let _ = io::stdout().flush();
let mut input = String::new();
io::stdin()
.read_line(&mut input)
.expect("Failed to read line");
input.trim() == "y"
}
I understand what's happening with the code. We're returning either true or false depending on the value of input, but I'm just curious as to why I should do it this way rather than my original code. They both do the same thing. Is the refactor more idiomatic or best practice? Just trying to understand. Thank you.
Several reasons, as you get more experienced that kind of boolean check and return becomes visual noise, and sometimes it just plainly causes logic bugs because you actually switched what you wanted to return.
Hi and thank you. I'll def use clippy from here on out. In fact I'll start going through the docs on it now. Very helpful to know you can also run it in the playground, bc I use that when I'm away from my computer.
Each language has idioms. Be it English, Pascal, Italian, Java or Rust. There are ways to express things that are common. It doesn't mean that other ways are wrong! But if you do things different than usual, the reader gets puzzled and ask himself why!? Did he want to say something different?
Learning idioms (or phrases or patterns) is key to learning a language. And so programming languages have developed their idioms that even span different "dialects".
You'll learn that by reading other's code and critically look at your's "Can I make that shorter". "Is there a simpler way?"
Getting fluent in a language is like starting to translate Spanish to Hungarian with the help of a dictionary.
Note that you changed the behavior of this function during your refactor. In the original version, inputs that are neither y nor n will be considered true but they will be considered false in the refactored version.
Neither of these is a particularly appealing choice; a better option would be to ask the user again if they gave you an unexpected response. Something like this, maybe:
fn get_bool_input(prompt: &str) -> bool {
loop {
print!("{prompt}");
let _ = io::stdout().flush();
let mut input = String::new();
io::stdin()
.read_line(&mut input)
.expect("Failed to read line");
return match input.trim() {
"y" => true,
"n" => false,
other => {
println!("Please respond with 'y' or 'n' (got {other:?}).");
continue;
}
};
}
}
You can also move the return inside the match, which might be easier to read for some people, or any one of a number of other variants:
match input.trim() {
"y" => { return true; }
"n" => { return false; }
other => { println!("Please respond with 'y' or 'n' (got {other:?})."); }
};
I wrote a new Rust version of this as I would if I expected to continue developing it into a larger, more flexible quiz system. It almost certainly uses more advanced techniques than you’ve covered yet, and is overkill if you don’t need the program to ever do more than it currently does, but I thought you might want to see one direction things might go in.
(link only, in case you want to avoid spoilers…)
Once small suggestion. get_int_input should take its inputs by value (in this case by copy), not by reference. So the signature should be fn get_int_input(_: i32, _: i32) -> i32. The reason for that is, that i32 is Copy (can be cloned by bitwise-copy) and is small (4 bytes). Copy values smaller than 2-3 words usually are better just passed by copy since they will fit in the registers.
Oh wow! What a difference! OK, now it is overkill, but I see and respect that you wanted to demonstrate that you are open to suggestions and want to learn.
Why would you ever want to write if <boolean_expr> { true } else { false }? That is always equivalent with just <boolean_expr> itself. Why all the excessive if-else? What is it for?
You shouldn't be writing code for the sake of writing more code. That makes zero sense.
By the way, get_this_input() and get_that_input() indicates that you are repeating yourself: you are doing the same thing with different types. That should be a generic function.
Now, not all programming language quirks can be analogized to natural languages, but for your particular question it could help.
Your prompt said, "Want to answer more questions? y or n:".
But your code reads:
if input == "n" {
return false;
}
else {
return true;
}
Which would translate to "Say no if you don't want to answer more questions."
You wouldn't say that in fluent English. If you say that, you're making things harder for yourself.
Instead, you say "Want to answer more questions?" by writing:
return input == "y"
The same logic apply in any programming language. Don't make your code confusing by saying things in roundabout ways. Don't make your code longer by adding redundant words. Make your code as short and straightforward as possible.
Thank you for your reply. I see what you mean. In my original code, any input besides "n" yields a true value and in the suggested refactor, only "y" yields a true value. I do really like the match construct you suggested. I just opened the Rust book to that section to go through it once again. Thank you.
Thank you for your feedback. I understand your point. i32 implements the Copy trait, so it lives on the stack and is easy make copies, and is not owned if passed into a function.