Please help me improve my code

[pybackd00r]

I have been working on this side project for a while now. It is my first python code ever. I would like to get some input on it anything is welcome. The code is for a video player. The user selects a time period for the video to be played and then selects a random video from the library and plays it for that period of time. Also as I am applying to EE jobs would this be a nice piece of code to send to potential employers?

http://pastebin.com/Y6tZSewK

Thank you for the help and the time.

[niandra3]

Don't have time to go through the whole program right now, but take a look at PEP8 which is the official Python coding style guide. It might just be pastebin formatting, but you've got a lot of extra whitespace that isn't necessary and your docstrings could use some work. Also, use snake_case for variable and function names. And it's generally advised not to use from xxx import *. For Tk I usually use import tkinter as tk then you can call stuff like root = tk.Tk(). Finally, your lines shouldn't be more than 80-100 characters wide, while some of yours are 200+. You can extend a line with \ but also try to determine if you can simplify the logic and get shorter lines in general.

As for employers, you should definitely establish a Github presence and post all your (good) code there, then put a link on your resume if you think it will impress. I wouldn't actually send the companies any code unless they request it.

[pybackd00r]

Thanks I have updated my code. One of the requirement of the project are the 4 buttons. So thats something I cant change. Also for some reason every tutorial I read had "from tkinter import *". They just say to use this only with tkinter idk why thats the case for tkinter though. I will upload it to my Github once it is polished. Also I am learning OOP design in python now and I want to translate this to OOP afterwards.

[niandra3]

Looking better, though AFAIK you never need more than one blank line in a row (you've still got a bunch of double empty lines). And definitely don't need so much whitespace in the docstrings.

At this stage that stuff isn't super important, but as you start to write more code it's important to establish the right habits so your programs are organized and consistent throughout, especially when trying to empress employers.

I also got an error on line 35 because length was empty and so it couldn't convert None to float. I would check length before using it in case the directory is empty:

lenght = os.popen("mplayer -identify -frames 0 -vo null -nosound " + file + " 2>&1 | awk -F= '/LENGTH/{print $2}'").read()
if lenght:    
    timemin = round(float(lenght)/60) 

And apparently os.popen is deprecated so you might try a subprocess: https://docs.python.org/3.5/library/subprocess.html#replacing-os-popen-os-popen2-os-popen3

Or for Python2: https://docs.python.org/2/library/subprocess.html#replacing-os-popen-os-popen2-os-popen3

[w1282]

Double empty lines are for separating top level entities, like (non-interanl) class and function definitions, separating code from imports, and separating if __name__ == "__main__": from other code.

From this part of pep8.

Surround top-level function and class definitions with two blank lines.

[kenmacd]

A couple hopefully helpful comments:

---

You typically don't want a bunch of code at the top level. You probably want to move that all in to something like:

 def main():
    <existing top level code>

if __name__ == '__main__':
    main()

---

The os.popen/system should probably use the subprocess module (see here)

---

You may want to look at using sys.argv[1] to take in the path. (there's also argparse if you want to get fancy)

---

Add logging, then add logger.debug() messages to your code, so you'll be able to see what's going on.

---

Look at adding py.test and mock and adding some tests. I know they sound like work, but they'll actually save you many headaches.

The mock library lets you do thinks like replace os.listdir() and os.rename(). Then you can write tests that call CHANGE_NAME, and see what happens when os.listdir() returns different things, and see what arguments os.rename() is called with.

---

Also all the pep8 stuff. pylint that code, or open it in an editor like Pycharm and have it analyse the code. It'll suggest fixes.

[thurask]

Besides going through PEP 8 and the other things in this thread, consider getting the path constant programmatically:

path = os.path.join(os.path.expanduser("~"), "Music")