Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Project Review #4

Open
VehvilainenPooki opened this issue Dec 1, 2023 · 0 comments
Open

Project Review #4

VehvilainenPooki opened this issue Dec 1, 2023 · 0 comments

Comments

@VehvilainenPooki
Copy link

Project Review

Your Code looks very clean and concise. You have divided everything into modules classes and functions very well. There is one problem. The code is mostly filled with one character variables.

One character variables

As your project is mostly mathematics it feels like you have followed pseudocode quite closely and your code is therefore unreadably dense and filled with one character variables. When comparing polring.py to most other modules, it feels like you had a different processes when making it. The variables have descriptive names and the code feels more readable. If the one character variable had descriptive names it would instantly make the code more readable.

Architecture documentation

Probably even more important would be the creation of architecture documentation. This project has many modules and it is not obvious how they work together. Creating documentation describing what all the modules do and how they interact with each other would most likely be a massive help in understanding the complete project.

Current documentation

The requirements.md is very well written that explains the project very well.

The tests.md is quick and concise. Would any of these questions add to the document: Do you feel like the test suite is complete? Would you like to add any other tests? Is there any tests that could be improved?

Usage guide wasn't great to use. It has this large code block that could have been split into multiple parts and explained in parts, but now is described in one paragraph. I don't know how the .txt files should look like so I didn't manage to use the commands correctly and get a readable output.

Minor notes

  • If I am not mistaken, requirements.md has a typo in the first paragraph. When you say " that will then be used in faster asymmetric encryption." you mean symmetric?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant