search

nickspeal / musicThisWeek

A Web Service that generates a Spotify playlist of bands that are playing in your area in the near future
3 stars 2 forks source link

Ed/add fields to artist [WIP] #45

Open esmason opened 8 years ago

esmason commented 8 years ago

note [WIP] before this can be merged need to:

-fix broken tests -discuss design ?refactor of backend

esmason commented 8 years ago

issue is that the functionalities of looking up artist URIs and finding top tracks are kind of separated in SpotifySearcher so had to create more instance variables to bridge the gap (because I wanted to minimize lookups and saves)

nickspeal commented 8 years ago

I recommend creating an Artist object with the attributes you want (Name, URI, Top tracks, etc). You can instantiate this object in the filter_list_of_artists method (though that name should probably change - it's so poor for describing its functionality).

You could replace the filter_artist function with one or more methods in the Artist class to look up the URI.

Take a look at the structure of eventFinder.py (specifically the Event object and the buildEvents() method) for design inspiration.

This new functionality probably warrants significant rework to the structure of this file. Don't worry about bending over backwards too hard to make it fit into the initial (prototype) design

nickspeal commented 8 years ago

Ignore my inline comments - I think creating an Artist class is the way to resolve all the issues

esmason commented 8 years ago

yep I thought about that, seemed like less efficient perfomance wise because we are going to end up dealing with a shit ton of artists and there's added overhead for each instantiation but it does seem appealing as far as design, encapsulation and readability are concerned. and it minimizes DB writes to the same extent AND it's robust to new fields added to the artist model in the future

I'm happy to do it this way, it makes a lot of sense

esmason commented 8 years ago

I've completed the refactoring of spotifyHandler in accordance with what I wrote on slack. It works on my computer and holds up to manual testing (tried about 10-20 different searches).

The tests are failing because the api for spotifyHandler has changed a fair bit.

Before I update the test_spotify file completely (changed two classes so far) I'll wait for your comments on this refactoring @nickspeal .