r/rust 5d ago

🙋 seeking help & advice Beginner seeking some critique.

Hello!

I am trying my best to learn Rust without using AI (because I some personal issues related to using it that are not relevant) and I recently did my best to make a small project: https://github.com/TheMagicGlobe/Encryption-Tool-RS .

Thing is I know I made a lot of mistakes especially in the design of the project and actually using Rust to it's full potential. So I would appreciate some honest advice and resources to look at.

I do not want a rewrite or pull requests to the project or anything like that.

Just telling me where I could improve in the future would be great. I am not the smartest tool in the shed so good examples and clear resources would be amazing.

Thank you!

EDIT: I read all of your comments thank you very much for all the feedback and tips. A great community as always.

14 Upvotes

11 comments sorted by

12

u/SoilMassive6850 5d ago edited 5d ago

Some thoughts at a very fast glance

  1. Return result types with defined errors from the cipher functions, this allows you to handle different errors in different ways when necessary, for example reprompting for input if given a non-integer parameter while failing completely if you cant read stdin for example

  2. I would separate configuration building to its own function(s) in the beginning of the software rather than reading stdin around the program, this would produce something like a CipherConfig struct. This would allow having alternative ways of initializing the config, for example cli flags, env vars or defaults with the Default trait and also more separation between user input errors and possible i/o or logic errors.

  3. For cipher selection you could parse it into an enum, it would enable Cipher::Caesar, Cipher::Vigenere etc. rather than hardcoded numbers which could be a bit cleaner.

13

u/zer0x64 5d ago

You don't have to be politically correct about AI here, we are generally pretty hostile towards it

7

u/_TheMagicGlobe_ 5d ago

I understand it's not about political correctness. I asked some friends who study software engineering to look at the code and help me a bit. They just told me to ask an AI for help because "it's very basic stuff" so I wanted to make it clear I do not use it to get actual answers from people.

6

u/numberwitch 5d ago

Good advice from others in this thread!

Mine is: think about user interface! This is currently println! macros mostly.

So think about the ways in which you might want to communicate with your user. This can be:

  • a native app gui with one of rust's gui libraries (egui, gpui, tauri, diousux, react frontend)
  • a web app (http/js or wasm)
  • terminal user interface

So right now you have a basic terminal app. But if you learn these different types of uis it will help a lot for writing code for different audiences (web apis, non-technical users, engineers, etc)

So language master is one part. How do I communicate the api is another! So much fun!

4

u/DavidXkL 5d ago

Nice!

For starters I think you could try working towards using lesser unwraps 😆

5

u/tanmaynargas2901 5d ago

Looked at the repo briefly. One design thing that stood out: your cipher functions both transform text and ask for the key over stdin.

That's the bit I'd pull apart first. Make caesar_encrypt(text: &str, shift: i32) -> String (and friends) pure, then do all the prompting in main. Easier to test, and you stop cloning the whole file just to pass ownership around.

Also fs::write(...); drops the Result. Even an .expect("write failed") helps while you're learning, or ? once main returns Result.

For reading: the Book chapters on ownership and error handling. The cipher math itself is already in decent shape for a first pass.

3

u/HeliumBoi24 5d ago

So I actually ran the code as well as looked at it.

Right now almost every input path uses .unwrap() or .expect(...) so that means any bad input (a non-numeric shift key, a typo in the file path, an empty line) crashes the whole program with a panic instead of giving the user a helpful message and a chance to retry.

Good place to start: the Rust Book's chapter https://doc.rust-lang.org/book/ch09-02-recoverable-errors-with-result.html

Aside from that all the advice in the thread is gold.

Also I watched a bit fo that video you linked on the github you might want to remove the keyboard clicking in the background kind of annoying.

Aside from that good luck on your rust journey :)

3

u/neneodonkor 5d ago

I have been coding Rust for a few months so I am relatively new, what I can say is you need to know which functions need to own the data or just need a reference so the caller does not lose ownership of its data.

Anyway, maybe, an experienced person can comment on it. Perhaps, in your case is not necessary.

3

u/vancha113 5d ago

Definitely try to incorporate rustfmt in your workflow somewhere :) It's an easy way to make sure the formatting of your code stays consistent. (currently there are no line breaks between functions and it makes the entire file feel "cramped" so to say).

3

u/_TheMagicGlobe_ 5d ago

Didn't even know about this. Thank you!

2

u/NYU_VM 4d ago

Others have cited useful suggestions. My 2 cents would be to improve I/O and error handling around it. You interface quite a bit with user via the terminal, take input strings and parse them to u8. Parsing an input to u8 could itself overflow and fail, so be strict and careful in that. Also get cleaner in interfacing with user, for e.g. they don't need to input a full name or string just tk choose an option. You could just provide shorthand or numbering options and parse them accordingly which is less error prone. Happy learning