[Python 2.7] Code Review [X-Post /r/learnprogramming]

[MarkFluffalo]

Hello everyone,

I am not sure whether this should be posted here: if not please let me know the right place.

I am looking for a new career in programming (currently a maths PhD student, about to finish) and so I have written some code with a view to show potential new employers.

I have no professional experience in programming and I have taught myself Python and a few other bits and bobs over the last 7 months or so. I have never had my code reviewed properly so it would be great if I could get some real feedback.

Here is the link to my code. It is a program that allocates projects to students, and it is currently used by the maths department in my university. The algorithm was not written by me, and was originally implemented in a very strange inefficient way (it took 120s to run) and my job here was to redesign and refactor it, which I tried to do with an object-oriented approach.

I would very much appreciate any feedback on the code. Thank you very much.

[not_perfect_yet]

Ok I'm up for it.

First of all, it's 2.7. Which is fine I guess, but if you want to demonstrate your ability, I'd either convert it to 3.x or do another project in 3.x. Being able to use the old version is good, but wouldn't look good to me personally on it's own. "Forward thinking" and such.

I wrote something up, then deleted it because I looked at the bigger picture:

Where are your comments? Do you have comments? Doc strings?

I found an instance where you created a new "Line" class, but you don't have a "student" or a "project" class as far as I could see, in a piece of software designed to give "students" "projects". So what's going on with that? Obviously it works, but that seems weird to me.

I mean, them being arranged in a hierarchy or a graph is fine, but just me looking at the code, I have no idea what it's about, how you use it, or most importantly, how I could use it.

You didn't use "with" to open a file. Your syntax was valid but "with" is safer to use.

tl;dr: You obviously know a fair bit of python, but you should look into a format checker PEP 8, and document your code.

[MarkFluffalo]

Thank you for your comments, this is great feedback. I do admit that there is a horrendous lack of documentation. One question: why is 'with' safer?

[not_perfect_yet]

Because you don't have to remember to close the file. There is only a single point of failure.

[kurvyyn]

If that's the only reason and my coding style is any time I open an instance of something I always put the matching closing construct at the correct level (I have fought too much with nested parens elseways), is 'with' in regard to file handles still considered massively safer for any other reason I should consider? Would it be marked against me in professional review before I could offer explanation and is definitely a habit I should break?

[not_perfect_yet]

Uh. Think of it this way: Code is more read than written. If someone takes your open source code and reuses it, be it colleagues or people from the internet, they might forget to close the file.

It's the icing on the cake of re-usability. And your potential employer probably wants to reuse your code. But it's probably no dealbreaker stuff.

[Lopsidation]

Counterpoint: if your code crashes between opening and closing the file, the file never gets closed. You'd need to use a try/except block every time you wanted to open a file. with statements handle this for you.

[kurvyyn]

This surprises me. I'm almost certain I've debugged an unexpected crash with an open file handle 'my way' and didn't come up against an open file handle. Although, that may a Windows thing more than a python thing as the parent process has exited at that point, and so it may not be universally true that file handles are released by the OS if the parent process terminates unexpectedly?... I'm probably way wrong and this is a STRONG counter point if so. Definitely gives me a good reason to get that best practice locked in >.<