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

MP3 Vote Collator - Final

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

Problem

Here is the completed code for my first Python project, for which I received some help and feedback here: Collate votes on MP3 files to list them by popularity

Seeing as there have been considerable changes since that post, I figured it would be best to start a new post.

My goals when starting this project were:

  • Scan sub folders for MP3 files (named in a predetermined format).



  • Sum the total amount of votes per song and output the results to a CSV file.



  • Create a list of who voted for which songs.



  • Copy those MP3 files to a single sub directory and rename them based on the results. For example, if a song received a total of 18 votes and came in at 11th position, it would have '11.' prepended at the start. This allows the MP3 files to be imported straight into a playlist in the correct order for a countdown.



I have completed these goals and my tests so far have worked perfectly. I am not totally satisfied with the final function, process_mp3. It seems a bit long-winded and could be improved, so I am looking for some feedback on that in particular.

```
import os, csv, operator, itertools, shutil
from collections import Counter, defaultdict

print("Welcome to the MP3 Vote Collator" + '\n')

# Global variables and lists
votes = defaultdict(list)
tally = Counter()
folder_list = os.getcwd()
base_dir = os.getcwd()

# The tally_votes function is responsible for searching for all .mp3 files, retrieving the vote count for each song and
# storing the results in the 'tally' counter.

def tally_votes():
print("STAGE 1. Processing Votes..." + '\n')
ignore_dir = ["Playlist"]
for subdir, dirs, files in os.walk(folder_list):
dirs[:] = [d for d in dirs if d not in ignore_dir]
for file in files:
if file.endswith(".mp3"):
file_name = file.split('.')[1] # Removes the .mp3 extension and numbers at the start of the file to clean up song name
vote_count = file.split('.')[0] # Retrieves vote coun

Solution

Docstring

Describing a function should not go over it as a comment but after it as a docstring:

def tally_votes():
   """
   The tally_votes function is responsible for searching for all .mp3 files,
   retrieving the vote count for each song and
   storing the results in the 'tally' counter.
   """


This way the user will read it when issuing help(tally_votes)

Return values and arguments

In short do not write:

def f():
   return x + 1


but:

def f(x):
   return x + 1


That is, give parameters to your functions.

For example process_result() should get the filename as an argument.

Constants

What if you decide that you know store music in ".waw" format? Looking for all the occurrences of ".mp3" and replacing them is sub-optimal.

So at the top of your file write

MUSIC_EXTENSION = ".mp3"


To simplify future changes.

Let the names tell, not the comments

Maintaining and upgrading both code and comments is double the effort than just comments.

For example:

_, tail = os.path.split(subdir) # Retrieves the name of the voter from the folder name


Becomes:

_, voter_name = os.path.split(subdir)


Given the more specific name, the comment is not needed anymore.

Tuple unpacking + avoiding temporaries

file_name = file.split('.')[1] # Removes the .mp3 extension and numbers at the start of the file to clean up song name
            vote_count = file.split('.')[0] # Retrieves vote count for each song
            song = file_name.lower() # Lower-casing prevents song name match issues


Becomes:

vote_count, song = file.lower().split('.')


Much cleaner.

Avoid except:

Always specify the exception when you use except.

Code Snippets

def tally_votes():
   """
   The tally_votes function is responsible for searching for all .mp3 files,
   retrieving the vote count for each song and
   storing the results in the 'tally' counter.
   """
def f():
   return x + 1
def f(x):
   return x + 1
MUSIC_EXTENSION = ".mp3"
_, tail = os.path.split(subdir) # Retrieves the name of the voter from the folder name

Context

StackExchange Code Review Q#113930, answer score: 5

Revisions (0)

No revisions yet.