Refactoring

README.md

@@ -1,5 +1,5 @@
 # SpotifyWeb
-This plugin connects to your [Spotify](https://www.spotify.com/) account and polls for the current track. It then displays the `track title` in the `status bar` of [Sublime Text](https://www.sublimetext.com/). The song is only displayed if Spotify is not paused/stopped.
+This plugin connects to your [Spotify](https://www.spotify.com/) account and polls for the current track. It then displays the `track` in the `status bar` of [Sublime Text](https://www.sublimetext.com/). The track is only displayed if Spotify is not paused/stopped.
 
 It is possible to Enable/Disable the plugin and also change the polling interval.
 

SpotifyWeb.sublime-settings

@@ -4,8 +4,8 @@
   "SpotifyWeb_bool_is_enabled": false,
   "SpotifyWeb_int_redirect_port": 8080, // 1024..65535
   "SpotifyWeb_int_refresh_interval_in_seconds": 5, // 2..15
-  "SpotifyWeb_bool_show_every_artist_name_instead_of_various_artists": true,
-  "SpotifyWeb_bool_music_name_comes_before_artist_name": true
+  "SpotifyWeb_bool_should_display_every_artist_name": true,
+  "SpotifyWeb_bool_should_display_title_before_artist": true
 }
 
 /*
@@ -15,6 +15,6 @@
      Make sure to click on the "add" button, when adding the Redirect URI.
   2. Copy and paste the client id and secret (and potentially the port) to your settings file.
   3. Set "SublimeSpotify_bool_is_enabled": true
-  
+
   Report issues or ask questions here: https://github.com/DevInsideYou/SpotifyWeb/issues
 */

src/spotify/Client.py

@@ -19,7 +19,7 @@ def __init__(self, client_id, client_secret, redirect_port):
       cache_path = os.path.abspath(
         os.path.join(
           os.path.dirname(__file__),
-          "../../../.SpotifyWeb-Cached-Token"
+          "../../../.spotifyweb-cached-token"
         )
       )
     )
@@ -45,45 +45,64 @@ def __extracted_token(self, token_response):
     else:
       None
 
-  def currently_playing_track_name(self, token, settings_manager):
-    def currently_playing_track_name(track, settings_manager):
+  def currently_playing_track(self, token, settings_manager):
+    def render_title_and_artist_or_show(title, artist_or_show_name):
+      if settings_manager.should_display_title_before_artist():
+        return title + " - " + artist_or_show_name
+      else:
+        return artist_or_show_name + " - " + title
+
+    def render_artists(artists):
+      number_of_artists = len(artists)
+
+      if number_of_artists == 1:
+        return artists[0]["name"]
+      elif not settings_manager.should_display_every_artist_name():
+        return "Various Artists"
+      else:
+        artist_name = ""
+
+        for index, artist in enumerate(artists):
+          artist_name += artist["name"]
+
+          if number_of_artists == (index + 1):
+            continue
+          else:
+            artist_name += ", "
+
+        return artist_name
+
+    def render_item(item):
+      if item is None:
+        return ""
+      else:
+        title = item["name"]
+
+        if item["type"] == "episode":
+          return render_title_and_artist_or_show(title, item["show"]["name"])
+        else:
+          return render_title_and_artist_or_show(title, render_artists(item["artists"]))
+
+    def currently_playing_track(track):
       if track is None:
         return ""
       elif track["is_playing"]:
-          if track["item"] is not None:
-            item = track["item"]
-            if item['type'] == 'episode':
-              return self.__returnResponse(item["name"], item['show']['name'], settings_manager)
-            else:
-              artistName = ""
-              if len(item["artists"]) == 1:
-                artistName += item["artists"][0]["name"]
-              elif settings_manager.show_every_artist_name_instead_of_various_artists():
-                for index, artists in enumerate(item["artists"]):
-                  artistName += artists["name"] 
-                  if len(item["artists"]) == (index + 1):
-                    continue
-                  else:
-                    artistName += ' - '
-              else:
-                artistName = 'Various Artists'
-
-          return self.__returnResponse(item["name"], artistName, settings_manager)
-
-    return currently_playing_track_name(
-      track = self.__get_current_track(token),
-      settings_manager = settings_manager
+        return render_item(track["item"])
+      else:
+        return ""
+
+    return currently_playing_track(
+      track = self.__get_current_track(token)
     )
 
   def __get_current_track(self, token):
-    return self.__get(token, "https://api.spotify.com/v1/me/player/currently-playing", {'additional_types': 'episode,track'})
+    return self.__get(
+      token,
+      "https://api.spotify.com/v1/me/player/currently-playing",
+      {
+        "additional_types": "episode,track"
+      }
+    )
 
   def __get(self, token, url, args=None):
     return SpotipyClient.Spotify(auth = token)._get(url, args)
-
-  def __returnResponse(self, trackParam, artistParam, settings_manager):
-    if settings_manager.music_name_comes_before_artist_name():
-      return trackParam + ' - ' + artistParam
-    else:
-      return artistParam + ' - ' + trackParam
-

src/spotify/SettingsManager.py

@@ -85,32 +85,32 @@ def are_credentials_at_least_partially_empty_or_none(self):
 
     return id is None or id == "" or secret is None or secret == ""
 
-  def show_every_artist_name_instead_of_various_artists(self):
+  def should_display_every_artist_name(self):
     default = False
 
     try:
-      return self.__show_every_artist_name_instead_of_various_artists(default)
+      return self.__should_display_every_artist_name(default)
     except:
       return default
 
-  def __show_every_artist_name_instead_of_various_artists(self, default):
-    actual = self.__read("SpotifyWeb_bool_show_every_artist_name_instead_of_various_artists")
+  def __should_display_every_artist_name(self, default):
+    actual = self.__read("SpotifyWeb_bool_should_display_every_artist_name")
 
     if actual:
       return actual
     else:
       return default
 
-  def music_name_comes_before_artist_name(self):
+  def should_display_title_before_artist(self):
     default = True
 
     try:
-      return self.__music_name_comes_before_artist_name(default)
+      return self.__should_display_title_before_artist(default)
     except:
       return default
 
-  def __music_name_comes_before_artist_name(self, default):
-    actual = self.__read("SpotifyWeb_bool_music_name_comes_before_artist_name")
+  def __should_display_title_before_artist(self, default):
+    actual = self.__read("SpotifyWeb_bool_should_display_title_before_artist")
 
     if actual is False:
       return actual

src/spotify/Spotify.py

@@ -50,7 +50,7 @@ def side_effect_current_track_name(token, fallback):
 
       def try_side_effect_current_track_name(valid_token):
         try:
-          self.__side_effect(client.currently_playing_track_name(valid_token, settings_manager))
+          self.__side_effect(client.currently_playing_track(valid_token, settings_manager))
         except Exception as e:
           self.__side_effect(str(e))
 
@@ -72,7 +72,7 @@ def side_effect_current_track_name_using_fresh_token(spotify_redirect_response):
         except:
           side_effect_error()
 
-      def side_effect_error():  
+      def side_effect_error():
         self.__side_effect(
           "SpotifyWeb could not get the authorization token from Spotify. "
           "Another tab should have opened in your browser for you to try to login again. "

tests/testSpotify.py

@@ -22,7 +22,7 @@ class FakeClient:
       def get_cached_token(self):
         return "some fake token"
 
-      def currently_playing_track_name(self, token, settings_manager):
+      def currently_playing_track(self, token, settings_manager):
         return "some track name"
 
     client = FakeClient()
@@ -52,7 +52,7 @@ def get_cached_token(self):
       def get_fresh_token(self, spotify_response_code):
         return "some fake token"
 
-      def currently_playing_track_name(self, token, settings_manager):
+      def currently_playing_track(self, token, settings_manager):
         return "some track name"
 
     client = FakeClient(redirect_port = 1337)
@@ -67,7 +67,7 @@ def side_effect(message):
 
     def send_oauth2_request(oauth2_url):
       self.assertEqual(oauth2_url, "some url")
-      
+
     def get_redirect_response(send_oauth2_request, oauth2_url, redirect_port, handle, available_duration_for_login_in_seconds):
       self.assertEqual(oauth2_url, "some url")
       self.assertEqual(redirect_port, 1337)
@@ -78,12 +78,12 @@ def get_redirect_response(send_oauth2_request, oauth2_url, redirect_port, handle
     spotify.run_once(client, send_oauth2_request, get_redirect_response, settings_manager)
     self.assertEqual(actual, "some track name")
 
-  def test_if_currently_playing_track_name_raises_an_exception_the_message_should_be_delegated_to_the_side_effect_function(self):
+  def test_if_currently_playing_track_raises_an_exception_the_message_should_be_delegated_to_the_side_effect_function(self):
     class FakeClient:
       def get_cached_token(self):
         return "some fake token"
 
-      def currently_playing_track_name(self, token, settings_manager):
+      def currently_playing_track(self, token, settings_manager):
         raise Exception("exception message")
 
     client = FakeClient()