My tkinter music player program

Question

This music player app hasn't really completed yet, but it runs the basic functions of a music player, which include play/pause, volume adjustment, next/prev, shuffle and repeat functionalities. However, the code is running under a single giant class that handles the entire program. It is extremely messy and further implementing the features will just lead to more mess. I mean, workable, but messy code. For this mess, I'm thinking of maybe separate the workings of some methods into another file, that or I create classes. I would really appreciate if anybody could check it for me. I'm open to listen to any suggestion or improvements that could help improve the structure of my code.

import os
import tkinter as tk
from tkinter import Button, filedialog as fd
from tkinter import ttk
from random import randint
from PIL import Image, ImageTk
from pygame import mixer
import pygame


# Constant variable to check whether or not current track has ended
END_OF_MUSIC = pygame.USEREVENT
mixer.music.set_endevent(END_OF_MUSIC)


class MusicPlayer(tk.Tk):
    def __init__(self) -> None:
        tk.Tk.__init__(self)
        pygame.init()
        mixer.init()

        self.music_end = END_OF_MUSIC

        self.icons = {}

        self.is_music_paused = False
        self.music_playing = False
        self.is_on_repeat = False
        self.is_mute = False
        self.is_load_file = False
        self.file_extension = ()

        self.track_index = 0
        self.track_info = None

        self.song_list = []
        self.song_file_paths = []
        self.file_info = {}
        self.file_indices = 0

        # Value of current volume
        self.sound_volume = 0

        # Value of previous volume
        self.previous_volume = 0

        self.song_directory = None
        self.track_count = 0

        self.is_shuffle = False

        self.is_track_repeat = False
        self.is_playlist_repeat = False

        self.configure_window()
        self.create_menubar()
        self.create_widgets()

    def configure_window(self):
        # Add a titlebar
        self.title("My Music Player")

        # Set background color
        self.configure(background="#212125")

        # Set the height and width of the window
        self.window_width = 1280
        self.window_height = 800

        # Set window geometry and center the window
        self.screen_width = self.winfo_screenwidth()
        self.screen_height = self.winfo_screenheight()
        self.x_coordinate = int((self.screen_width / 2) - (self.window_width / 2))
        self.y_coordinate = int((self.screen_height / 2.5) - (self.window_height / 2.5))
        self.geometry(
            f"{self.window_width}x{self.window_height}+{self.x_coordinate}+{self.y_coordinate}"
        )

        # Make window not resizable
        self.resizable(0, 0)

    def create_menubar(self):
        # Create a main menu
        self.menubar = tk.Menu(self, tearoff=False)

        # Create file menu under main menu
        self.filemenu = tk.Menu(self.menubar, tearoff=False)

        # Add a separator above the menu
        self.filemenu.add_separator()

        # Add submenus under main menu
        self.filemenu.add_command(
            label="Open...", font="Segoe 10", command=self.browse_file
        )
        self.filemenu.add_command(
            label="Open Folder...", font="Segoe 10", command=self.browse_directory
        )

        # Add a separator above the "Exit" command
        self.filemenu.add_separator()

        self.filemenu.add_command(label="Exit...", font="Segoe 10", command=exit)

        # Add file menu to main menu
        self.menubar.add_cascade(label="File", font="Segoe 10", menu=self.filemenu)

        # Configure main menu to root window
        self.config(menu=self.menubar)

    def create_widgets(self):
        self.create_bottom_frame()
        self.create_buttons()
        self.create_volume_slider()

    def create_bottom_frame(self):
        self.bottom_frame = tk.Frame(bg="silver")
        self.bottom_frame.grid(row=1, column=0, sticky="ew")
        self.bottom_frame.grid_columnconfigure(6, minsize=50)
        self.bottom_frame.grid_rowconfigure(0, weight=1)
        self.grid_rowconfigure(0, weight=1)
        self.grid_columnconfigure(0, weight=1)

    def browse_directory(self):
        self.file_extension = ('mp3', 'wav', 'ogg',)
        self.song_directory = fd.askdirectory(initialdir=os.getcwd(), title="Select Directory With Songs")

        if self.song_directory:
            self.create_playlist_panel()
            self.shuffle_button.config(state=tk.NORMAL)
            self.previous_button.config(state=tk.NORMAL)
            self.next_button.config(state=tk.NORMAL)
            self.play_button.config(state=tk.NORMAL)
            self.repeat_button.config(state=tk.NORMAL)

        self.load_songs()

    def load_songs(self):
        for file_index, song_filename in enumerate(os.listdir(self.song_directory)):
            song_path = os.path.join(self.song_directory, song_filename)
            if song_filename.endswith(self.file_extension):
                self.track_count += 1
                self.song_list.append(song_filename)
                self.playlist_box.insert(tk.END, f"{self.track_count}. {song_filename}")

                self.file_info[file_index] = [song_filename, song_path]

            self.file_indices += 1

    def browse_file(self):
        filetypes = [("Audio Files", ".mp3 .ogg .wav")]

        self.music_file = fd.askopenfilename(
            initialdir=os.getcwd(), title="Select A Song", filetypes=filetypes
        )

        if self.music_file:
            self.play_button.config(state=tk.NORMAL)
            self.repeat_button.config(state=tk.NORMAL)

        mixer.music.load(self.music_file)

    def load_icons(self):
        self.icons = {
            "shuffle_icon": ImageTk.PhotoImage(Image.open("./icons/shuffle.png").resize((30, 30))),
            "previous_icon": ImageTk.PhotoImage(Image.open("./icons/previous.png").resize((30, 30))),
            "next_icon": ImageTk.PhotoImage(Image.open("./icons/next.png").resize((30, 30))),
            "play_pause_icon": (ImageTk.PhotoImage(Image.open("./icons/play.png").resize((30, 30))),
                                ImageTk.PhotoImage(Image.open("./icons/pause.png").resize((30, 30)))),
            "repeat_icon": (ImageTk.PhotoImage(Image.open("./icons/repeat.png").resize((30, 30))),
                            ImageTk.PhotoImage(Image.open("./icons/repeat_one.png").resize((30, 30)))),
            "volume_icon": (ImageTk.PhotoImage(Image.open("./icons/mute.png").resize((30, 30))),
                            ImageTk.PhotoImage(Image.open("./icons/low_volume.png").resize((30, 30))),
                            ImageTk.PhotoImage(Image.open("./icons/medium_volume.png").resize((30, 30))),
                            ImageTk.PhotoImage(Image.open("./icons/max_volume.png").resize((30, 30))))}

    def create_buttons(self):
        self.load_icons()

        self.shuffle_button = Button(self.bottom_frame, relief=tk.FLAT, width=55, height=55, image=self.icons["shuffle_icon"], bg="silver", state=tk.DISABLED, highlightthickness=0, command=self.toggle_shuffle)
        self.shuffle_button.grid(row=0, column=0, padx=65, pady=15)

        self.previous_button = Button(self.bottom_frame, relief=tk.FLAT, width=55, height=55, image=self.icons["previous_icon"], bg="silver", state=tk.DISABLED, highlightthickness=0, command=self.play_previous_track)
        self.previous_button.grid(row=0, column=1, padx=65, pady=15)

        self.play_button = Button(self.bottom_frame, relief=tk.FLAT, width=55, height=55, image=self.icons["play_pause_icon"][0], bg="silver", state=tk.DISABLED, highlightthickness=0, command=self.toggle_play_pause)
        self.play_button.grid(row=0, column=2, padx=65, pady=15)

        self.next_button = Button(self.bottom_frame, relief=tk.FLAT, width=55, height=55, image=self.icons["next_icon"], bg="silver", state=tk.DISABLED, highlightthickness=0, command=self.play_next_song)
        self.next_button.grid(row=0, column=4, padx=65, pady=15)

        self.repeat_button = Button(self.bottom_frame, relief=tk.FLAT, width=55, height=55, image=self.icons["repeat_icon"][0], bg="silver", state=tk.DISABLED, highlightthickness=0, command=self.toggle_repeat)
        self.repeat_button.grid(row=0, column=5, padx=65, pady=15)

        self.audio_mute_button = Button(self.bottom_frame, relief=tk.FLAT, width=55, height=55, image=self.icons["volume_icon"][0], bg="silver", highlightthickness=0, command=self.toggle_mute_unmute)
        self.audio_mute_button.grid(row=0, column=8, pady=15)

    def toggle_shuffle(self) -> None:
        if self.is_shuffle:
            self.shuffle_button.config(relief=tk.FLAT, highlightthickness=0)
            self.is_shuffle = False
        else:
            self.shuffle_button.config(relief=tk.SUNKEN)
            self.is_shuffle = True

    def create_playlist_panel(self) -> None:
        # Create a frame for the playlist listbox
        self.playlist_frame = tk.Frame(self)
        self.playlist_frame.grid(row=0, column=0, sticky="w", padx=15)

        # Create a listbox for the playlist panel
        self.playlist_box = tk.Listbox(self.playlist_frame, bg="#111", fg="White", width=60, height=40, highlightthickness=0, relief=tk.FLAT)
        self.playlist_box.grid(row=0, column=0)

        # Create a vertical scrollbar for the playlist panel
        self.vertical_scrollbar = tk.Scrollbar(self.playlist_frame, orient=tk.VERTICAL)
        self.vertical_scrollbar.grid(row=0, column=1, sticky="ns")

        # Create a horizontal scrollbar for the playlist panel
        self.horizontal_scrollbar = tk.Scrollbar(self.playlist_frame, orient=tk.HORIZONTAL)
        self.horizontal_scrollbar.grid(row=1, column=0, sticky="we")

        # Attach the vertical and horizontal scrollbars to the listbox
        self.playlist_box.config(yscrollcommand=self.vertical_scrollbar.set)
        self.playlist_box.config(xscrollcommand=self.horizontal_scrollbar.set)

        # Set the listbox's scrollbar property to "yview" for vertical and "xview" for horizontal
        self.vertical_scrollbar.config(command=self.playlist_box.yview)
        self.horizontal_scrollbar.config(command=self.playlist_box.xview)

    def start_playlist_music(self):
        self.track_info = self.file_info[self.playlist_box.curselection()[0]]
        self.play_button.config(image=self.icons["play_pause_icon"][1])
        mixer.music.load(self.track_info[1])
        mixer.music.play()

    def play_previous_track(self):
        try:
            self.track_index = self.playlist_box.curselection()[0] - 1
            # Clear active bar in the playlist panel
            self.playlist_box.selection_clear(0, tk.END)
            # Set the active bar to the next song in the playlist
            self.playlist_box.selection_set(self.track_index)
            # Activate the new song bar
            self.playlist_box.activate(self.track_index)
            self.start_playlist_music()
        except IndexError:
            # Set the active bar to the last track in the playlist
            self.playlist_box.selection_set(tk.END)
            # Activate new selection bar for the last track in the playlist
            self.playlist_box.activate(tk.END)
            self.start_playlist_music()

    def play_next_song(self) -> None:
        if self.is_shuffle:
            self.track_index = randint(0, self.playlist_box.size() - 1)
            self.playlist_box.selection_clear(0, tk.END)
            self.playlist_box.selection_set(self.track_index)
            self.playlist_box.activate(self.track_index)
            self.start_playlist_music()
        else:
            try:
                self.track_index = self.playlist_box.curselection()[0] + 1
                # Clear active bar in the playlist panel
                self.playlist_box.selection_clear(0, tk.END)
                # Set the active bar to the next song in the playlist
                self.playlist_box.selection_set(self.track_index)
                # Activate the new song bar
                self.playlist_box.activate(self.track_index)
                self.start_playlist_music()
            except IndexError:
                self.playlist_box.selection_set(0)
                self.playlist_box.activate(0)
                self.start_playlist_music()
    
    def check_event(self):
        self.music_end = pygame.USEREVENT
        pygame.mixer.music.set_endevent(self.music_end)

        for event in pygame.event.get():
            if event.type == self.music_end:
                self.play_next_song()
        
        self.after(500, self.check_event)

    def play_music(self) -> None:
        if self.song_directory is not None:
            self.check_event()
            self.start_playlist_music()
        else:
            mixer.music.play()

        self.play_button.config(image=self.icons["play_pause_icon"][1])
        self.music_playing = True

    def pause_music(self) -> None:
        mixer.music.pause()
        self.play_button.config(image=self.icons["play_pause_icon"][0])
        self.is_music_paused = True

    def unpause_music(self) -> None:
        mixer.music.unpause()
        self.play_button.config(image=self.icons["play_pause_icon"][1])
        self.is_music_paused = False

    def toggle_play_pause(self) -> None:
        if not self.music_playing:
            self.play_music()
        else:
            if self.is_music_paused:
                self.unpause_music()
            elif not self.is_music_paused:
                self.pause_music()

    def toggle_repeat(self) -> None:
        if self.is_playlist_repeat:
            self.repeat_button.config(relief=tk.SUNKEN, image=self.icons["repeat_icon"][1])
            self.is_track_repeat = True
            self.is_playlist_repeat = False
        elif self.is_track_repeat:
            self.repeat_button.config(relief=tk.FLAT, image=self.icons["repeat_icon"][0])
            self.is_playlist_repeat = False
            self.is_track_repeat = False
        else:
            self.repeat_button.config(relief=tk.SUNKEN)
            self.is_playlist_repeat = True

    def set_volume(self, volume: str) -> None:
        self.sound_volume = float(volume)

        if self.sound_volume == 0.0:
            self.audio_mute_button.config(image=self.icons["volume_icon"][0])
        elif 0.25 >= self.sound_volume <= 0.5:
            self.audio_mute_button.config(image=self.icons["volume_icon"][1])
        elif 0.5 >= self.sound_volume <= 0.75:
            self.audio_mute_button.config(image=self.icons["volume_icon"][2])
        else:
            self.audio_mute_button.config(image=self.icons["volume_icon"][3])

        mixer.music.set_volume(self.sound_volume)

    def create_volume_slider(self):
        self.volume_bar = ttk.Scale(
            self.bottom_frame,
            from_=0, to=1,
            orient=tk.HORIZONTAL,
            length=150,
            command=self.set_volume)

        # Set the initial volume bar to 0.5
        self.volume_bar.set(0.5)
        # Grid the volume bar
        self.volume_bar.grid(row=0, column=9, padx=15)

    def toggle_mute_unmute(self):
        if not self.is_mute:
            # Store the value of volume before muting
            self.previous_volume = self.sound_volume

            mixer.music.set_volume(0.0)
            self.volume_bar.set(0.0)
            self.audio_mute_button.config(image=self.icons["volume_icon"][0])
            self.is_mute = True
        else:
            mixer.music.set_volume(self.previous_volume)
            # Set volume bar back to previous volume
            self.volume_bar.set(self.previous_volume)
            self.is_mute = False


if __name__ == "__main__":
    root = MusicPlayer()
    root.mainloop()

Again, the program works fine, just that the problem I'm having is this one giant class handling all the functionalities of the app.

You can divide into classes, e.g, one class for menus, one class for progress bar, one class for deciding which song to play next, hence, by using separate classes, you can introduce several features like ads, ai captioning, and other things like that easily without breaking other stuffs. — Vansh Patel
– Vansh Patel, Commented Mar 11 at 14:10
I agree that it can be messy right now in dividing into classes, and sometimes maybe you break things up, but once you did it perfectly, it would save you a lot of hectic in future. (Based on my personal experience.) — Vansh Patel
– Vansh Patel, Commented Mar 11 at 14:16

Sara J · Accepted Answer · 2025-03-11 15:50:19Z

This is an interesting one, because it looked very neat at a glance, but the deeper I looked the more things I found troubling, not just in terms of structure, but also in terms of whether or not the code works at all. It turns out that for all but the simplest use cases it doesn't.

But I'll save that part for later and go over some other things first.

What state do we need, and where do we need it?

One of the first things that stands out to me is that almost all data is attached to the MusicPlayer at all times - exactly one of these 24 functions takes a parameter other than self (set_volume, which in addition to self also has a volume parameter. Which is, confusingly, a str). They contain a grand total of one local variable between them - everything else, including things that are set once, never modified, and never needed outside the function that defines them get bolted onto the MusicPlayer instance.

Now, this isn't formally wrong, but it means that it's hard to tell at a glance what state exists, let alone what functionality depends on what state. And this seems to have caused some issues, which we'll get to.

Additionally, nothing ever resets! We set values, and then those values stick around even when they probably shouldn't. As an example, the indices in the playlist view keep going up every time the user opens a new directory, even though only the tracks from the last directory are in there (e.g. if the user open a directory with 5 tracks, then another with 3 tracks, they'll have a playlist containing 3 tracks numbered 6-8). self.song_directory also has related issues, which we'll get to in a moment.

There's also several instance variables (is_on_repeat, is_load_file) which are defined but never used.

One feature, two implementations

One part of the architecture I don't like is how there are basically two entirely different usage flows in this app - the "play a single file" flow and the "open a directory" flow - whose code barely overlaps at all despite conceptually doing very similar things. Having two entirely disparate implementation of the same feature not only risks causing conflicts between them (and we will get to that soon), but also just seem to make the code longer and more complicated for what appears to be little benefit

`browse_file`

I consider this method badly broken.

First, because it can only be used once. If a file has been played before, we unload it and load a new file. But there's no way to start playing the new file - since self.music_playing has been set and can never be unset (because nothing ever gets reset), the play button will only unpause already-playing music - which there is none, as we've never told the mixer to start playing the newly-loaded file (and we have no way of doing so).

Second, because it can't be used after the directory mode has been used. If we use this function after a directory has been opened, self.song_directory is set (because nothing ever gets reset), so all controls ignore the file we just opened in favor of the directory.

And third, less importantly, because closing the dialogue window without picking a file makes a stack trace show up in my console.

`browse_directory`

This method is ill-behaved for directories containing both music and non-music files (and/or music files in unsupported formats). Attempting to open such a directory causes file_info and song_list to get out of sync. Which means problems like the wrong track playing, or KeyErrors. For a concrete example, consider what happens if a user opens a directory containing the following files:

cover.jpg
Neon.flac
Neon.mp3
Illusion.flac
Illusion.mp3
Wanderlust.flac
Wanderlust.mp3

Now, what does file_info end up containing in this case? Turns out the answer is { 2: "Neon.mp3", 4: "Illusion.mp3", 6: "Wanderlust.mp3" }. And those three files are the ones that show up in the playlist view, as expected. If the user then tries to play Wanderlust, which is on the third row in the playlist, we try to play file_info[2]. Which is Neon, which is the wrong track. And if the user had asked to play Illusion, which is on the second line of the playlist, we'd try to play file_info[1], which isn't a file at all but a KeyError. Neither of those are ideal.

I also want to point out that pressing play after opening a directory does nothing except print a KeyError to the console (hint: curselection() returns an empty tuple if nothing is selected). This is awkward from a usability perspective.

Finally, if the user opens a different directory while a track is already playing, that track will keep playing, even though it is no longer visible in the playlist (because nothing ever gets reset) - which isn't too bad, but a bit unexpected.

Window setup

The setup process has a part that I find strange - as far as I can tell, create_bottom_frame, create_buttons and create_volume_slider are very interdependent, and there's very little to make that clear. Like, sure, if I accidentally change the constructor in a way that makes it call create_buttons before create_bottom_frame I will soon find out that that isn't a good idea, but why is that a mistake that I'd even have the opportunity to make? If calling create_buttons without having access to a bottom frame should be impossible, why not enforce that by making create_buttons take the frame in which the buttons should be created as a parameter? And if calling create_bottom_frame without calling create_buttons afterwards is useless, why not make create_bottom_frame itself call create_buttons instead of relying on the programmer always remembering it?
The same goes for create_volume_slider, and these changes would end up making the create_widgets function redundant

Next and prev

play_previous_track doesn't respect is_shuffle, which seems a bit strange. But more importantly, this function and play_next_song (why does one refer to "tracks" and the other to "songs" by the way?) are completely identical except for the shuffle handling (which doesn't necessarily seem like it should differ between the two) and a single + being replaced with a -. Consider breaking the common part out into some kind of shared utility function.

Alternatively, if prev shouldn't respect shuffle, it should probably not be available when is_shuffle is true.

toolic · Accepted Answer · 2025-03-11 10:53:25Z

Comments

Many of the comments are useful, but some of them can be removed to reduce clutter. For example, all of the following comments can be deleted since they merely restate what is obvious in the code:

    # Add a titlebar
    self.title("My Music Player")

    # Set background color
    self.configure(background="#212125")

    # Set the height and width of the window
    self.window_width = 1280
    self.window_height = 800

Documentation

The PEP 8 style guide recommends adding docstrings for classes and functions. The class docstring could include a description similar to the text of the question, where it lists the features.

DRY

This path ./icons/ is used repeatedly. Set it to a variable, and that will make it easier to change.

Simpler

When I see an if/else with a not in it:

if not self.is_mute:
else:

I prefer to invert the polarity to make it easier to understand:

if self.is_mute:
else:

This means you need to swap the lines in each branch as well.

Also:

if self.song_directory is not None:
else:

Partitioning

It is extremely messy

I disagree with your assessment. However, it is good that you are trying to think of a way to separate some code out of your one big class before adding features. Think about whether any of the new features overlap with current features, and if there would be code duplication, then that might be motivation for another class.

I slightly disagree about what you think regarding Partitioning. I personally think that dividing into multiple classes would help creating advanded features in future easily without breaking other stuff. Yeah, it can be messy right now, but in future, for new features, it would be very convinient. — Vansh Patel
– Vansh Patel, Commented Mar 11 at 14:07

Toby Speight · Accepted Answer · 2022-04-12 07:03:01Z

-2

First I want to compliment you on your very understandable code.

I have the same question for a tkinter app of my own.

I found this post on stack Overflow to be useful.

I had 2 classes contained in my original app. Here's what I did.

I copied one of them, class VerticalScrolledFrame(tkinter.Frame) into a file named code\codeReviewExamples\vscroll_frame.py
I added a __init__.py file to the directory to enable modules.
In the original BIG file I:
1. commented out class VerticalScrolledFrame
2. added import vscroll_frame
3. changed the definition of self.frame
  - was: self.frame = VerticalScrolledFrame(root)
  - now: self.frame = vscroll_frame.VerticalScrolledFrame(root)

edited Apr 12, 2022 at 7:03

Toby Speight

88.4k14 gold badges104 silver badges327 bronze badges

answered Apr 5, 2022 at 17:17

Love and peace - Joe Codeswell

973 bronze badges

2

\$\begingroup\$ I feel this could be better posed as a comment, rather than an answer. If you have any specific code review points, please add them! \$\endgroup\$

Ben A
– Ben A

2022-04-05 18:30:58 +00:00
Commented Apr 5, 2022 at 18:30
3

\$\begingroup\$ Other than the code is very readable do you have any other insightful observations about the original code that you could make? Please read How do I write a good answer? \$\endgroup\$

pacmaninbw
– pacmaninbw ♦

2022-04-06 12:13:53 +00:00
Commented Apr 6, 2022 at 12:13
\$\begingroup\$ Hi @pacmaninbw , No I don't have any other comments at this time. \$\endgroup\$

Love and peace - Joe Codeswell
– Love and peace - Joe Codeswell

2022-08-15 19:51:24 +00:00
Commented Aug 15, 2022 at 19:51

Add a comment |

Stack Exchange Network

My tkinter music player program

3 Answers 3

What state do we need, and where do we need it?

One feature, two implementations

`browse_file`

`browse_directory`

Window setup

Next and prev

Comments

Documentation

DRY

Simpler

Partitioning

You must log in to answer this question.

Hot Network Questions

3 Answers 3

What state do we need, and where do we need it?

One feature, two implementations

browse_file

browse_directory

Window setup

Next and prev

Comments

Documentation

Simpler

Partitioning

You must log in to answer this question.

Related

`browse_file`

`browse_directory`