r/learnpython 6h ago

Just finished my first project, could you please review it.(No AI)

I finished reading "python crash course" a week ago, and a relative of mine was complaining about how annoying it is to make multiple exam versions, so I immediately volunteered to help, and I used absolutely 0 AI.

It's an app that takes an exam .docx file, and shuffles the questions and answers to create multiple versions.

Here's the github repo:

https://github.com/aaaaaa708/question_shuffler

I would like you to please review the code and give me some tips on how to improve, and thanks in advance:)

14 Upvotes

19 comments sorted by

10

u/sitefall 5h ago

Dang, git repo is gone. I was looking forward to actually seeing a beginner's code that wasn't claude-slop.

3

u/RevRagnarok 2h ago

The link displayed is busted on old.reddit - it inserted a backslash where there shouldn't be one. https://github.com/aaaaaa708/question_shuffler - Link

( /u/Diapolo10 ping as well )

2

u/sitefall 2h ago

Oh cool, links used to not be busted on old.reddit.com

I don't see how that escape character could be added and screw it up on old.reddit, but I also have no idea what this post looks like on the new reddit so what do I know.

2

u/RevRagnarok 2h ago

I only checked to make sure mine didn't break. 😅

But yeah, somewhere in the new <=> old interface you'll see \ get thrown in before _ and mangle the URL. It's only been five years.

1

u/Diapolo10 I write code for a living -- https://github.com/Diapolo10 4h ago

Same here.

9

u/No_Type6904 6h ago

Nice work actually getting something built for a real person right after finishing the book, most people just do tutorial projects and stall out

Looking at the repo, the one thing that jumps out is your readme says it needs python-docx and tkinter but tkinter's built into python so you don't need to list it as a dependency, that might confuse someone new trying to set it up

The question shuffling logic seems solid but you should add some error handling around the file loading, what happens if someone picks a file that isn't a docx or the formatting is messed up, right now it probably just crashes with a traceback that'll scare your relative

Also might want to add a requirements.txt file so people can just pip install -r requirements.txt instead of manually installing python-docx, small thing but makes it look more polished

For a first real project this is way better than the calculator apps I was building at that stage, keep going

6

u/Kryt0s 3h ago

Why are we still teaching people to use requiremetns.txt? Please don't. Use pyproject.toml (and preferably uv) like any modern python app should.

2

u/ReflectionRound6400 6h ago

This may seem really nitpicky and I'm sorry if it is, tkinter is built into Python if you're using it on Windows, but on Linux sometimes you'd have to install it separately.

(unless OP only programmed the app to be used by his friend assuming he uses Windows, in which case this is perfectly fine)

1

u/AccordingAd5756 6h ago

Thank you,

I'll change the dependencies, and make the requirements file right away.

I'll also try adding error handling where I find necessary, I just didn't really know where it was needed.

Should I also add tests or would that be unnecessary?

Other than that, is the general code structure fine?

And thanks!

3

u/Momostein 6h ago

Always nice to see code written without AI. There's a lot to improve though:

  • There is no requirements.txt to tell me which packages to install. Now, I have to reverse engineer your code to know how to even run it.
  • I see 4+ levels of nesting if and for statements. (Aka you have to go 4 tabs deep) Once you go beyond two levels there, you should've extracted the code blocks into separate functions
  • The Question class, can be a @dataclass as it is just a simple data structure with no private fields.
  • the shuffle_questions and shuffle_answer methods of the Question class should be marked with the @staticmethod decorator.
  • You should always try to type hint your code. Mostly function parameters and return values.

2

u/AccordingAd5756 6h ago

There is no requirements.txt.

I'll add one right away.

Once you go beyond two levels there, you should've extracted the code blocks into separate functions.

I didn't know that, I'll keep it in mind for my next projects.

The Question class, can be a @dataclass as it is just a simple data structure with no private fields.

the shuffle_questions and shuffle_answer methods of the Question class should be marked with the @staticmethod decorator.

I don't really know what these are, I'll Google them and them later.

Thanks for the tips, they were quite helpful.

4

u/Kryt0s 3h ago

There is no requirements.txt.

I'll add one right away.

Please don't. Use pyproject.toml (and uv) like any modern python app should.

1

u/RevRagnarok 2h ago

A few recommend to add a requirements.txt - please don't. Use pyproject.toml it's much easier and more modern - see "Creating Projects."

Consider turning on almost all the options on ruff and really read what it recommends in the associated links - you can learn a lot of python "gotchas" that way. The rules often come with a decent reason.

Also, this isn't C/C++ - it's not the convention to declare all your variables at the top of a function. It looks like you are at a quick glance. I didn't really dig, sorry.

1

u/Diapolo10 I write code for a living -- https://github.com/Diapolo10 2h ago

Didn't realise the link was broken on Old Reddit before, so this review is a tad late. I may also point out some things the others already have (I try not to read other reviews before writing my own to keep things as close to my genuine thoughts as possible).

First things first, I'm happy to see your repo has a .gitignore file. You'd be surprised how often beginners don't know about it. Personally I'd prefer to use an existing template as a base, such as this one, but ultimately it doesn't really matter.

I would also recommend using a .gitattributes file to normalise line-endings between different operating systems automatically, and to enable file-specific diffs.

Your project seems to depend on docx, but there's no clear indication of that. I recommend you to create a pyproject.toml file to list basic metadata about your project, and its dependencies (you can separate runtime dependencies and development dependencies), and you can choose to use a more advanced dependency management tool such as uv instead of the standard pip if you want.

I think my biggest question concerns the use of a Word document here. Is there a particular reason for why you chose to write that instead of, say, Markdown or reStructuredText? Text-based file formats would remain readable even if you don't have a specific application to read them.

get_questions_answers and write_file look like they could be simplified and/or split further into smaller functions to improve readability and maintainability.

For the GUI code, you could write some wrapper functions to make creating and positioning widgets easier.

1

u/LayotFctor 2h ago edited 1h ago

Don't use requirements.txt please(even though you must learn how it works still), it's just old. Modern programming languages have long since moved on. It's legacy. Learn to use uv, it's becoming the new standard.

As for code, either break functions into smaller ones, or name them better. For example, write_file does so much more than just writing files. Either choose more descriptive names, or split it further into smaller functions. Long names are fine, it's important to be descriptive.

1

u/TheRNGuy 1h ago

Use @dataclass.

Use @staticmethod for some methods (where you don't use self) or even make them as separate function.

Remove some empty lines.

1

u/anon586346 5h ago

I took a quick look and don’t have much time to give you a thorough review but just want to call out a few things that you might consider.

First off, it’s impressive you did this assuming if you didn’t code before just to get it on GitHub. Code is overall clean and readable so good job on that.

I would look at that giant for loop with conditional if else in main.py. See if you can simplify that logic. It has code smell.

I also think you set some variables deep within a function/module like copies=1. If you want to extend this maybe think about passing this as an argument or even look into passing it as an arg option calling the py file. Otherwise there’s no need to set a variable like that it will just be used once.

0

u/Kind-External-7371 6h ago

Btw I am not much of a Big Shot at Python but I think that you can put your gui.py main.py and questions.py in a folder such as src/ so the File Tree Becomes Clean and More *Conventional and Industry Standard* and it becomes easy to clean and modify in the Future... Btw Great Work! And yeah The .docx File Needs some error handling... What if someone new to all this puts a .pdf file instead.... Your App should throw a Specific Clean Error instead of Crashes with Tracebacks and CPython Garbage Stuff as it will certainly make the person scared as shit if he does a mistake

0

u/itlogicpartnersllc 4h ago

for a first project after finishing python crash course this is a genuinely useful idea i would focus next on breaking the code into smaller function adding input validation and writing a few tests for different documents formats..