patternpythonMinor
Random playlists program
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':
```
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:
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
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.
Here's one way to address these issues:
Global variables
Try to avoid global variables as much as possible.
For example, instead of this:
It would be better to make the function return the
And then adjust the callers appropriately to pass the number as parameter where needed, for example instead of:
Write:
Adjust the rest of the code accordingly, and try to eliminate the other global variables too,
For these two, another good alternative will be to create a class, where these variables will be member fields.
Simplifications
Instead of this:
A bit simpler way to write:
Optimizing imports, and coding style
The script is not actually using
And the style guide recommends to import one package per line, like this:
Do read the style guide, it has many other recommendations that apply to your script.
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)
breakGlobal 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 randomDo 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)
breakdef 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.