Hello,
some time ago I shared a Guess game that I made from scratch and this time I thought to do something different so I decided to make a Guess game over IRC.
It’s pretty basic but I learned a lot about destructing Enum, unwrap and if let syntax.
I would appreciate any suggestion :)
here is the source code:
use futures::prelude::*;
use irc::client::prelude::*;
use rand::Rng;
#[tokio::main]
async fn main() -> irc::error::Result<()> {
let config = Config {
nickname: Some("beep_boop".to_owned()),
server: Some("irc.libera.chat".to_owned()),
channels: vec!["#test".to_owned()],
..Default::default()
};
let mut client = Client::from_config(config).await?;
client.identify()?;
let mut stream = client.stream()?;
let mut secret_number = rand::thread_rng().gen_range(0..101);
let mut attempts = 0;
while let Some(message) = stream.next().await.transpose()? {
print!("{}", message);
match message.command {
Command::PRIVMSG(channel, message) => {
if let Some(command) = message.get(0..6) {
if command == "!guess" {
let parts = message.split(' ').collect::<Vec<_>>();
if let Some(part2) = parts.get(1) {
if let Ok(num) = part2.to_string().parse::<u32>() {
println!("{:?}", num);
if num == secret_number {
client.send_privmsg(&channel, format!("You won with {attempts} attempts! generating next number")).unwrap();
secret_number = rand::thread_rng().gen_range(0..101);
attempts = 0; // reseting the number of attempts
} else if num > secret_number {
client.send_privmsg(&channel, "too high").unwrap();
attempts += 1;
} else {
client.send_privmsg(&channel, "too low").unwrap();
attempts += 1;
}
}
}
}
}
else {
continue;
}
if message.contains(client.current_nickname()) {
client.send_privmsg(channel, "beep boop").unwrap();
}
},
_ => println!("IRC command not implemented: {message}"),
}
}
Ok(())
}
Your code creeps to the right very quickly. This makes it hard to read and follow. Best to avoid that and avoid too many levels of indentations. Typically when you have:
if condition_1 { if condition_2 { if condition_3 { actual_work(); } } }You can refactor it to:
if !condition_1 { continue; // or return or some other way to short circuit } if !condition_2 { continue; // or return or some other way to short circuit } if !condition_3 { continue; // or return or some other way to short circuit } actual_work();This generally makes things easier to follow, you can check all the validation conditions at the start then forget about them for the actual body. With the nested ifs you generally need to keep them in mind until you know there is no else block or anything else which just adds to the cognitive load while reading the code. Especially in your case with some code and come continues below it means you need to jump up and down between the start and the end trying to understand what code gets run in different conditions.
However in your case you are using if lets which makes things a bit more complex but you can now (in edition 2021 since rust version 1.65) do this instead of the if let:
let Some(command) = message.get(0..6) else { continue; }This is known as the let-else syntax.
You are also throwing away a lot of errors with this approach. This can be annoying to the user if they format things wrong or do something unexpected you just ignore the message - which could make it seem like the bot is not working. Instead of telling the user they did something wrong and should try again which is much more user friendly.
There is a major usability issue with the bot as well if you ever get more than one user trying to interact with it at once. They will share the same guess and attempts and when one person wins those will be reset causing a huge amount of confusion to the second player. You likely want to store the guesses and attempts in a hashmap on the channel id (I think at least since this is private messages each PM will have a unique ID? If not you need to base it on something unique to that interaction).
Your unwraps on sending the messages could also be an issue. If there is ever a problem with the connection your application will crash losing all in progress games/data. You may want to handle that a bit more gracefully and attempt to reconnect when that happens (ie have a loop around the connection code that keeps trying to connect with a delay between each attempt before trying to get messages in the inner loop.)
Thank you for the in depth suggestion :)
And more functions help as well. For example:
match message.command { Command::PRIVMESSAGE(channel, message) => { handle_message(&mut state, channel, message); } }stateholds all relevant state, and if you’re fancy, handle_message could return a future so it doesn’t block (e.g. if you need to update a database or read a file to handle a command; if you do this,stateneeds to be in a mutex).I try to separate dispatch from handlers to keep things tidy and testable.
If you use functions that return result (like your main, but with a different error) you could remove some of the if let blocks with
let num = part2.to_string().parse::()?;. That might obfuscate some of the conditionals so the statements are one after another.Cool project, let us know about the next iteration.
Although ending the program when an error is encountered for invalid user input is probably not desired behavior for an IRC bot.
The intent is to capture the parsing errors within a function, so the response could be capture in a single if let or match or is_okay condition.
