Background Info
I am an intermediate level Python programmer. This is my implementation of a MP3 Playlist class
challenge that was hosted on 101 Computing.Net. The goal of the challenge was to implement a Class in Python to maintain an MP3 Playlist. The key features of the class are the ability to:
- Enqueue an MP3 to the playlist
- Remove an MP3 from the playlist
- Load a playlist from a text file
- Save a playlist on a text file
- Shuffle all the songs in the playlist
- Display all the tracks from the playlist
- Count the number of tracks in the playlist
- Calculate the total duration of the playlist
- Empty/Reset the playlist
- Check if the playlist is empty
MP3PLaylist Class
This is the mp3playlist.py
file which contains the MP3Playlist class
.
import os
import sys
import datetime
from random import shuffle
from track import Track
class MP3PlayList:
"""A MP3Playlist class."""
def __init__(self, title: str, filename: str) -> None:
"""Initialize attributes."""
self.title = title
self.filename = filename
self.tracks = []
def __repr__(self) -> str:
"""String representation the class."""
return f'{self.__class__.__name__}(title={self.title}, filename={self.filename}, tracks={self.tracks})'
def load_playlist(self):
"""Loads a saved playlist, and displays its contents"""
if not os.path.isfile(os.path.join('music_files', self.filename)):
print(f'{self.title} could not be loaded.\n{self.filename} does not exists!', file=sys.stderr)
else:
with open(os.path.join('music_files', self.filename)) as file_obj:
song_list = [i.strip().split(',') for i in file_obj]
for song in song_list:
self.add_track(Track(song[0], song[1], song[2]))
self.show_playlist()
def save_playlist(self, filename=None):
"""Saves the current playlist in a textfile"""
self.filename = self.filename if filename is None else filename
with open(os.path.join('music_files', self.filename), 'w+') as file_object:
for track in self.tracks:
song, artist, duration = track.title, track.artist, track.duration
file_object.write(f'{song},{artist},{duration}' + '\n')
print(f'{self.title} has been successfully saved.')
def add_track(self, track):
"""Add a single track to the playlist."""
self.tracks.insert(0, track)
print(f'"{track.title}" has been added successfully!')
def remove_track(self, track_name):
"""Removes a track from the playlist"""
is_located = False
for i, track in enumerate(self.tracks):
if track_name == track.title:
self.tracks.pop(i)
print(f'"{track_name}" has been successfully removed.')
is_located = True
if not is_located:
print(f'"{track_name}" could not be located. Please try again.')
def show_playlist(self):
"""Displays entire playlist, including title, artist, and duration."""
if self.tracks:
print('Track list:')
for track in self.tracks:
song, artist, duration = track.title, track.artist, track.duration
print(f'\t• {song} - {artist} ({duration})')
else:
print('Track list is empty!')
def shuffle_playlist(self, n):
"""Shuffles playlist n x"""
if self.tracks:
for i in range(n):
shuffle(self.tracks)
print(f'Playlist was shuffled {n}xs!')
else:
print('Playlist could not be found.')
def get_number_of_tracks(self):
"""Prints the total number of tracks stored in the playlist."""
print(f'Total tracks: {len(self.tracks)}')
def get_total_duration(self):
"""Prints the total duration time of all tracks."""
current_year = datetime.datetime.now().year
datetime_original = datetime.datetime(year=current_year, month=1, day=1)
for track in self.tracks:
minutes, seconds = [int(i) for i in track.duration.split(':')]
datetime_original += datetime.timedelta(minutes=minutes, seconds=seconds)
print('Total duration: {:%H:%M:%S}'.format(datetime_original))
def reset_playlist(self):
"""Clears the playlist."""
self.tracks.clear()
print(f'{self.title} has been reset.')
def is_empty(self):
"""Returns True if playlist is empty, False otherwise"""
return len(self.tracks) == 0
Track Class
There is also a Track class
associated with the project. This is the track.py
file which contains the Track class
.
class Track:
"""A Track class"""
def __init__(self, title, artist, duration):
"""Initialize attributes."""
self.title = title
self.artist = artist
self.duration = duration
def __repr__(self):
return f'{self.__class__.__name__}(title={self.title}, artist={self.artist}, duration={self.duration} seconds)'
main.py
This is the main.py
file.
from mp3playlist import MP3PlayList, Track
def main():
# Initialize Playlist:
mp3_playlist = MP3PlayList('MyCoolMusic', 'music_playlist.txt')
# Check if empty:
print(mp3_playlist.is_empty()) # True
# Load playlist from textfile:
mp3_playlist.load_playlist()
# Remove a track by title
mp3_playlist.remove_track('one')
# Add a new track:
track = Track('metallica', 'the unforgiven', '6:23')
mp3_playlist.add_track(track)
# Informs user if file cannot be found:
mp3_playlist.remove_track('bogus_file') # File cannot be located!
# Shuffles list 3 x times:
mp3_playlist.shuffle_playlist(3)
# Get total number of tracks:
mp3_playlist.get_number_of_tracks()
# Get total duration of playlist
mp3_playlist.get_total_duration()
# Save Playlist and filename by name designated at instance initialization
mp3_playlist.save_playlist()
# Shuffle list 7x times:
mp3_playlist.shuffle_playlist(7)
# Create a file with a new custom file_name:
mp3_playlist.save_playlist('backup_copy.txt')
if __name__ == '__main__':
main()
music_playlist.txt
The data from music_playlist.txt
was used to build the Track
class, which is then loaded into the MP3Playlist
class.
cruel summer,bananarama,3:50
no ordinary love,sade,2:41
when doves cry,prince,3:45
jammin,bob marley,3:20
veridis quo,daft punk,5:58
hotel california,eagles,6:30
one,metallica,7:26
kunglim guli,yulduz usmanova,3:45
mea culpa,enigma,5:02
solitude,marion,4:11
tokyo,skeler,3:39
un simple histoire,thievery corporation,3:45
in the air tonight,phil collins,5:37
metallica,the unforgiven,6:23
The Project Directory Structure
This is the directory structure for the project. The text files
used in this project are stored in music_file
directory.
MP3Playlist/
├── music_files/
│ ├── music_playlist.txt
│ ├── backup_copy.txt
│
├── mp3playlist.py
├── track.py
├── main.py
Here is a link to my MP3 Playlist
project on GitHub.
Goals & Expectations
My goal is to be an expert level Python programmer someday. Therefore, tell me where I can improve my code. Any comment of constructive criticism are welcomed.
Questions
- Is the code organized, well documented, and easy to read? If not, where can I improve?
- Please analyze and critique my
load_playlist()
,save_playlist()
, andremove_track()
functions in particular. Are these functions written well? Could they be written better? Could you offer any code examples of you would have approached these functions? - Are there any other areas of my code that could be re-written more efficiently?
- This is my first code review; is there a question I should have ask?
-
\$\begingroup\$ Excellent first question \$\endgroup\$Reinderien– Reinderien2022年07月07日 11:47:57 +00:00Commented Jul 7, 2022 at 11:47
2 Answers 2
Your code in my opinion is well written and well organized. But to make it even better I have a few suggestions:
Even more type annotations
Although you've used type annotations but at some places it can be used to make your code more expressive. For example in __init__
of MP3PlayList
you can add type hint to tell the reader that self.tracks
is a list of Track
objects. Something like this:
self.tracks: list[Track] = []
Using data classes
The Track
class, AFAIK has no functionality. It just stores data about a special track. In this case you can use data classes. One of the main benefit of using them is that you no longer need to define __init__
or __repr__
by hand. Maybe the equivalent implementation of Track
class using data classes is this:
>>> from dataclasses import dataclass
>>>
>>> @dataclass
... class Track:
... title: str
... artist: str
... duration: int
...
>>>
>>>
>>> track = Track(title="Night Call", artist="London Grammar", duration=270)
>>> repr(track)
"Track(title='Night Call', artist='London Grammar', duration=270)"
>>>
>>> track
Track(title='Night Call', artist='London Grammar', duration=270)
>>>
Handling exceptions
If the music_playlist.txt
is not organized as your will or the file was corrupted your program crashes. So I think it is better to use try
statement in the load_playlist
method and handle the condition that the file is not in the correct format.
-
1\$\begingroup\$ Thank you for your feedback! I appreciate you showing me how to implement type hint for self.tracks list. I didn't realize you could do that ~very cool! I updated changes to my GitHub. Also, thank you for bringing dataclasses to my attention; I have a lot to learn! \$\endgroup\$Seraph776– Seraph7762022年07月08日 20:33:09 +00:00Commented Jul 8, 2022 at 20:33
-
\$\begingroup\$ @Seraph776 I hope it helps you :) \$\endgroup\$Amir reza Riahi– Amir reza Riahi2022年07月09日 05:27:52 +00:00Commented Jul 9, 2022 at 5:27
There's not much point to defining a __repr__
for these classes - that's intended for machine legibility, and you're more concerned with human-legible printing so should override __str__
instead. Along those lines, show()
should be changed so that it doesn't print, and its contents returned in __str__
.
All of your existing comments are less useful than having no comment at all. Things that should actually be commented include your file format.
load_playlist
being an instance member that mutates an existing instance of the class is not a good idea. Instead, consider writing a @classmethod
pseudoconstructor that loads the playlist into a new instance. Said in another way, why should we require that there be a useless, empty class instance before loading any file? It should be immediately populated.
Why should we force the use of the music_files
prefix? This can be added at the outer level by the caller.
Prefer pathlib.Path
instead of the os.path
methods.
Don't parse CSV yourself. What if a song or artist title has a comma in it? Python has a csv
module to do this for you safely.
Don't index into Track(song[0], song[1], song[2])
by integer. Do a positional tuple unpack instead.
None of the methods in your playlist class should print, as the class is a business logic class. Printing can occur at the outer level.
filename
should probably not be kept as a property of the class, and can just be a parameter to the load and save methods.
remove_track
can be simplified using a generator to which you apply either a next()
or a tuple unpack of a single element, depending on whether you allow duplicates or not. It should raise an exception rather than printing an error.
I don't see the point in shuffling a playlist multiple times. In theory, with a good enough random generator, you'll be entirely unable to distinguish whether a playlist has been shuffled once or a million times.
Your methods for the number of tracks, total track duration and empty checking can be converted into property methods.
is_empty
should be reused in your show()
logic.
Consider including the title of the playlist in your show()
logic.
Reliance on the current year and a full datetime
for what should only be timedelta
instances is pretty nasty. Stay in the timedelta
domain.
Suggested
from datetime import timedelta
from pathlib import Path
from random import shuffle
from typing import NamedTuple, Iterable, Sequence, Iterator
import csv
class MP3PlayList:
def __init__(self, title: str, tracks: list['Track']) -> None:
self.title = title
self.tracks = tracks
@classmethod
def load(cls, filename: Path, title: str) -> 'MP3PlayList':
"""
Load the playlist from a header-less CSV with format looking like:
artist name,track name,6:23
The track duration is in totalminutes:seconds (no hours).
"""
with filename.open() as file_obj:
reader = csv.reader(file_obj)
tracks = list(Track.from_rows(reader))
return cls(title=title, tracks=tracks)
def save(self, filename: Path) -> None:
"""
Save the playlist to a header-less CSV with format looking like:
artist name,track name,6:23
The track duration is in totalminutes:seconds (no hours).
"""
with filename.open('w') as file_obj:
writer = csv.writer(file_obj)
writer.writerows(Track.to_rows(self.tracks))
def add_track(self, track: 'Track') -> None:
self.tracks.insert(0, track)
def remove_track(self, track_name: str) -> None:
"""
Remove a track by name. The name must match exactly one track or a ValueError will be raised.
"""
i_track, = (
i for i, track in enumerate(self.tracks)
if track.title == track_name
)
self.tracks.pop(i_track)
def __str__(self) -> str:
if self.is_empty:
return f'Track list "{self.title}" is empty!'
return (
f'Track list "{self.title}":\n'
+ '\n'.join(
f'\t• {track}' for track in self.tracks
)
)
def shuffle(self) -> None:
shuffle(self.tracks)
@property
def track_count(self) -> int:
return len(self.tracks)
@property
def total_duration(self) -> timedelta:
return sum(
(track.duration for track in self.tracks),
start=timedelta(),
)
def reset_playlist(self) -> None:
self.tracks.clear()
@property
def is_empty(self) -> bool:
return len(self.tracks) == 0
def parse_duration(s: str) -> timedelta:
"""
Parse into a timedelta a string of form totalminutes:seconds
"""
minutes, seconds = s.split(':')
return timedelta(minutes=int(minutes), seconds=int(seconds))
def format_duration(delta: timedelta) -> str:
"""
Format a timedelta to a string of form totalminutes:seconds
"""
minutes, seconds = divmod(delta.total_seconds(), 60)
return f'{minutes:.0f}:{seconds:02.0f}'
class Track(NamedTuple):
title: str
artist: str
duration: timedelta
@classmethod
def from_rows(cls, rows: Iterable[Sequence[str]]) -> Iterator['Track']:
for title, artist, dur_str in rows:
yield cls(
title=title, artist=artist, duration=parse_duration(dur_str),
)
@staticmethod
def to_rows(tracks: Iterable['Track']) -> Iterator[tuple[str, ...]]:
for track in tracks:
yield track.title, track.artist, format_duration(track.duration)
def __str__(self) -> str:
return f'{self.title} - {self.artist} ({format_duration(self.duration)})'
def main() -> None:
parent = Path('music_files')
mp3_playlist = MP3PlayList.load(parent / 'music_playlist.txt', 'MyCoolMusic')
print(mp3_playlist)
mp3_playlist.remove_track(track_name='one')
track = Track('metallica', 'the unforgiven', timedelta(minutes=6, seconds=23))
mp3_playlist.add_track(track)
try:
mp3_playlist.remove_track('bogus_file')
except ValueError:
print('Track could not be located. Please try again.')
mp3_playlist.shuffle()
print(mp3_playlist.track_count)
print(mp3_playlist.total_duration)
mp3_playlist.shuffle()
mp3_playlist.save(parent / 'backup_copy.txt')
if __name__ == '__main__':
main()
Output
Track list "MyCoolMusic":
• cruel summer - bananarama (3:50)
• no ordinary love - sade (2:41)
• when doves cry - prince (3:45)
• jammin - bob marley (3:20)
• veridis quo - daft punk (5:58)
• hotel california - eagles (6:30)
• one - metallica (7:26)
• kunglim guli - yulduz usmanova (3:45)
• mea culpa - enigma (5:02)
• solitude - marion (4:11)
• tokyo - skeler (3:39)
• un simple histoire - thievery corporation (3:45)
• in the air tonight - phil collins (5:37)
• metallica - the unforgiven (6:23)
Track could not be located. Please try again.
14
1:04:49
-
1\$\begingroup\$ Thank you so much for taking the time to post your code, It looks absolutely professional! This is the greatest expectation I could have hoped for in a code review. Thank you again for all of your feedback! \$\endgroup\$Seraph776– Seraph7762022年07月09日 05:24:20 +00:00Commented Jul 9, 2022 at 5:24
Explore related questions
See similar questions with these tags.