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

Peer review #2 #2

Open
miartris opened this issue Feb 25, 2023 · 0 comments
Open

Peer review #2 #2

miartris opened this issue Feb 25, 2023 · 0 comments

Comments

@miartris
Copy link

Cloned 19.00 25.2

Instructions for set up worked for me, but I had to run the main.py file directly since invoke start with poetry failed. Although it might just be my environment this could be taken into account in user instructions.

The application looks awesome and I didn't encounter any bugs.

The program is structured nicely. Documentation of functions etc. is very extensive. In some cases it's probably fine to omit or trim some of it for compactness such as in a few simple getters in Markov Player since the code itself along with type hints is very self-documenting. The class also contains a try-except block with quite a few errors that seem to not be explicitly dealt with that could be made more explicit by either handling them or detailing why not.

There is a decent amount of file handling. Maybe enough to warrant extracting the logic to a dedicated Filehandler-class. Conventions are followed nicely here with reading .env values and performing filepath operations with os functions.

I can't comment much on the algorithmic side of the program since I'm not overly familiar when it comes to Markov chain generated stuff.

Excellent work overall!

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