Peer review

[ihakkin]

Repository was copied 18.6.2024 at 11.45.

Project seems interesting and overall appears to be in a good state. Here are some thoughts and suggestions.

rsa_functionality.py

• In generate_keys function there may be some unnecessary repetition.

Current implementation:

Suggested modification : This way there is no need to initialize variables outside the loop that might need to be recalculated anyway.

User Interface:

UI is not the main focus of the course, but here are some comments about it.

• When an incorrect public key or an invalid string is used for encryption, the error message is not very informative. This shouldn’t be a major problem if user ensures that fields are empty before pasting values, but error message is a bit confusing:

I also got the same error message when decrypting a longer (>150 characters) string.

• UI could be a bit more intuitive and it would be easier if the user could see the whole fields. The “Enter string to decrypt” field shows user the end of the string, and “copied to your clipboard” shows the beginning of the string.

Tests

• You might consider adding some test, e.g., test encryption and decryption with different lengths of messages (including an empty string).

I learned a lot while reviewing your project and studying the RSA algorithm more in depth. I hope some of my comments might be helpful.

[TheNushu]

Thank you so much for the feedback!

1. The suggested code modification seems like a nice one and I will implement it in the project.
2. Thanks for raising the issue it is weird that the same error message is on incorrect input and on "big" texts. I am guessing that something happens in the encryption that loses some data. I will investigate deeper.
3. Hmm, I think a fix of that should be to make the UI fullscreen. I was debating with myself if I should do it, but now that you also mentioned it, sounds like a wise decision.
4. Yes, the current level of testing wasn't fully covering because I had some problems setting up the tools and unittesting practice in general. There were some issues when I tried encrytion-decryption testing (which might be tied to the bug you found I would guess) that are now some of the top priorities of work for the project. :D

Thank you for the review once again!