marcderbauer / songcrawler

Crawl Spotify and Genius for all the songs of your favourite artist!
MIT License
0 stars 0 forks source link

Seperate into files? #35

Closed marcderbauer closed 1 year ago

marcderbauer commented 1 year ago

Could the Music class be seperated into smaller files?

I remember that there were difficulties regarding the spotipy dependency, but maybe there's a workaround?

Would be A LOT cleaner & more maintainable.

marcderbauer commented 1 year ago

Not just the music class, Songcrawler etc. would be great if I could split them up too.
Will also need to rethink the Path class being in utils. It doesn't fully fit in there as it's really closely associated with Songcrawler.

Maybe Music could be a folder? Songcrawler another? Main.py would be the only file on the top level

marcderbauer commented 1 year ago

Having some difficulties with a circular dependency in music_collection.init().

It was probably bad design, will figure this out once I have the rest of the refactoring working!

marcderbauer commented 1 year ago

Current circular dependency issue:

Music -> Song -> Album -> MusicCollection-> Song

Currently MusicCollection calls Song twice:

  1. To create song objects when reading albums from a file
  2. To create song objects when pooling an album / playlist -> Both are legitimate reasons to call the song class

Song needs the album to save correctly as it creates an album object in order to save. This is likely the one to change. Maybe this can be moved somewhere? Maybe write a seperate mechanism in request.request, where it saves the result? Could create an object there and pass the song from there on?

marcderbauer commented 1 year ago

Problem

I created a Song.to_album_dict() method which returns all the information necessary to construct an album from a song as dict. The idea was to remove the Album/MusicCollection dependency of when songs.
Instead, the song is prepared in an easy "export format", which the Album class can then handle and save.
Ideal workflow:

album = Album(**song.to_album_dict())
album.save(...)

The problem here is with overwriting. If there is an album, this method completely overwrites it to save a single song. What's missing is the part where it reads in the existing album and appends.

Idea

Create a method within Album that reads a single song and saves it.
IMPORTANT: Album can import the Song Class! -> Maybe no need for .to_album_dict()

  1. Take the Song (either as album dict, or ideally the way it is!)
  2. Look if there already is an album which is associated with the artist/song 2.1 If so: Read that album and append the song if it doesn't exist there yet 2.2 If not: Create a new album

I'm imagining something like Album.save_song()

marcderbauer commented 1 year ago

Implemented it as MusicCollection.save_song() as it made sense to use this for Playlists as well. Only thing left for music refactoring is the collection name (either at playlist or MusicCollection). TODO: figure out collection name #24

Currently refactoring Songcrawler and Request. Might have to remove the request class due to the circular dependency?

marcderbauer commented 1 year ago

Merged the Request class into songcrawler. Refactoring into separate files is done now.