HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Random playlists program

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
randomprogramplaylists

Problem

This is the first program I've created without help from a tutorial. Written in Python 3.4, it gathers a specified number of media files and opens them in the default player (I use VLC on Windows 8.1). Python is my first language I have been learning mostly through trial and error. I am posting here to see what I can improve and to learn as much as I can.

```
import os, random, sys

completed_media = (r"C:\Users\user_000\Desktop\Completed Media")

all_media = []
playlist_ = []

def create_media_list():
for path, subdirs, files in os.walk(completed_media):
for file in files:
if file.lower().endswith(('.mkv', '.mp4', '.divx', '.avi')):
all_media.append(os.path.join(path, file))

def random_selection_from_media():
random_selection = random.choice(all_media)
if random_selection not in playlist_:
playlist_.append(random_selection)
else:
pass

def get_selections():
for i in range(number):
random_selection_from_media()
print_playlist_()
playlist_confirmation()

def number_of_selections():
while True:
try:
global number
number = int(input('How many files would you like to add to playlist? >>> '))
break
except ValueError:
print('Enter a number.')

def print_playlist_():
print('\n-------In playlist-------\n')
print('[{0}]'.format('\n-------------------------\n'.join(str(i) for i in enumerate(playlist_, 1))))
print('\n-------End playlist------\n')

def remove_selection():
while True:
try:
to_remove = int(input('To remove a selection enter the number of the selection you want removed here. >>> '))
if to_remove >> ').lower()
if ok == 'yes' or ok == 'y':
play_playlist_()
elif ok == 'no' or ok == 'n':
while True:
new = input('Get new list or remove a specific selection? >>> ').lower()
if new == 'new list' or new == 'n':

Solution

Pretty good for a self-taught beginner, keep going like this and you're golden!

Recursion and infinite loop

This is probably the worst part of the script:

def remove_selection():
    while True:
        try:
            to_remove = int(input('To remove a selection enter the number of the selection you want removed here. >>> '))
            if to_remove <= len(playlist_):
                break
        except ValueError:
            print('Enter a number.')
            remove_selection()
        try:
            playlist_.pop((to_remove - 1))
            break
        except (IndexError, UnboundLocalError):
            print('Enter a vaild number')
            remove_selection()


An infinite loop, a misused exception handling,
an incorrect error handling, and recursive calls...
This is confusing and more complicated than it needs to be.

You can replace the recursive calls with continue.

Exception handling for converting the input to integer is appropriate, because there is no better way. It is not appropriate for validating the input is within range, because a better way exists using conditionals.

Also, it's not enough to check that the index is less than the length of the list, you also need to check that it's 0 or greater.

UnboundLocalError is thrown when trying to access a variable that hasn't been assigned. That's not an exception to catch, it indicates a problem in the logic that needs to be fixed.

Here's one way to address these issues:

while True:
    try:
        to_remove = int(input('To remove a selection enter the number of the selection you want removed here. >>> '))
    except ValueError:
        print('Enter a number.')
        continue
    if not (0 < to_remove <= len(playlist_)):
        print('Enter a valid number (within range: 1..{})'.format(len(playlist_)))
        continue
    playlist_.pop(to_remove - 1)
    break


Global variables

Try to avoid global variables as much as possible.

For example, instead of this:

def number_of_selections():
    while True:
        try:
            global number
            number = int(input('How many files would you like to add to playlist? >>> '))
            break
        except ValueError:
            print('Enter a number.')


It would be better to make the function return the number:

def number_of_selections():
    while True:
        try:
            return int(input('How many files would you like to add to playlist? >>> '))
        except ValueError:
            print('Enter a number.')


And then adjust the callers appropriately to pass the number as parameter where needed, for example instead of:

number_of_selections()
get_selections()


Write:

get_selections(number_of_selections())


Adjust the rest of the code accordingly, and try to eliminate the other global variables too, all_media and playlist_.
For these two, another good alternative will be to create a class, where these variables will be member fields.

Simplifications

Instead of this:

if ok == 'yes' or ok == 'y':


A bit simpler way to write:

if ok in ('yes', 'y'):


Optimizing imports, and coding style

The script is not actually using sys, so you can drop that import.

And the style guide recommends to import one package per line, like this:

import os
import random


Do read the style guide, it has many other recommendations that apply to your script.

Code Snippets

def remove_selection():
    while True:
        try:
            to_remove = int(input('To remove a selection enter the number of the selection you want removed here. >>> '))
            if to_remove <= len(playlist_):
                break
        except ValueError:
            print('Enter a number.')
            remove_selection()
        try:
            playlist_.pop((to_remove - 1))
            break
        except (IndexError, UnboundLocalError):
            print('Enter a vaild number')
            remove_selection()
while True:
    try:
        to_remove = int(input('To remove a selection enter the number of the selection you want removed here. >>> '))
    except ValueError:
        print('Enter a number.')
        continue
    if not (0 < to_remove <= len(playlist_)):
        print('Enter a valid number (within range: 1..{})'.format(len(playlist_)))
        continue
    playlist_.pop(to_remove - 1)
    break
def number_of_selections():
    while True:
        try:
            global number
            number = int(input('How many files would you like to add to playlist? >>> '))
            break
        except ValueError:
            print('Enter a number.')
def number_of_selections():
    while True:
        try:
            return int(input('How many files would you like to add to playlist? >>> '))
        except ValueError:
            print('Enter a number.')
number_of_selections()
get_selections()

Context

StackExchange Code Review Q#128431, answer score: 2

Revisions (0)

No revisions yet.