Re-apply old TR feedback

Co-authored-by: Bartosz <[email protected]>

Author: martin-martin

python-selenium/README.md

@@ -28,7 +28,7 @@ To run the music player, install the package, then use the entry point command f
 You'll see a text-based user interface that allows you to interact with the music player:
 
 ```
-Type: play [<track_number>] | tracks | more | exit
+Type: play [<track_number>] | pause | tracks | more | exit
 >
 ```
 

python-selenium/pyproject.toml

@@ -16,6 +16,3 @@ dependencies = [
 ]
 [project.scripts]
 bandcamp-player = "bandcamp.__main__:main"
-
-[tool.setuptools.packages.find]
-where = ["src"]

python-selenium/src/bandcamp/__main__.py

@@ -1,11 +1,9 @@
-from bandcamp.app.tui import TUI
+from bandcamp.app.tui import interact
 
 
 def main():
-    """Provides the main entry point for the app."""
-    tui = TUI()
-    tui.interact()
+    """Provide the main entry point for the app."""
+    interact()
 
 
-if __name__ == "__main__":
-    main()
+main()

python-selenium/src/bandcamp/app/player.py

@@ -4,11 +4,11 @@
 from bandcamp.web.element import TrackElement
 from bandcamp.web.page import DiscoverPage
 
-BANDCAMP_DISCOVER = "https://bandcamp.com/discover/"
+BANDCAMP_DISCOVER_URL = "https://bandcamp.com/discover/"
 
 
 class Player:
-    """Plays tracks from Bandcamp's Discover page."""
+    """Play tracks from Bandcamp's Discover page."""
 
     def __init__(self) -> None:
         self._driver = self._set_up_driver()
@@ -22,11 +22,11 @@ def __enter__(self):
         return self
 
     def __exit__(self, exc_type, exc_value, exc_tb):
-        """Closes the headless browser."""
+        """Close the headless browser."""
         self._driver.quit()
 
     def play(self, track_number=None):
-        """Plays the first track, or one of the available numbered tracks."""
+        """Play the first track, or one of the available numbered tracks."""
         if track_number:
             self._current_track = TrackElement(
                 self.tracklist.available_tracks[track_number - 1],
@@ -35,13 +35,13 @@ def play(self, track_number=None):
         self._current_track.play()
 
     def pause(self):
-        """Pauses the current track."""
+        """Pause the current track."""
         self._current_track.pause()
 
     def _set_up_driver(self):
-        """Creates a headless browser pointing to Bandcamp."""
+        """Create a headless browser pointing to Bandcamp."""
         options = Options()
         options.add_argument("--headless")
         browser = Firefox(options=options)
-        browser.get(BANDCAMP_DISCOVER)
+        browser.get(BANDCAMP_DISCOVER_URL)
         return browser

python-selenium/src/bandcamp/app/tui.py

@@ -1,76 +1,70 @@
 from bandcamp.app.player import Player
 
 
-class TUI:
-    """Provides a text-based user interface for a Bandcamp music player."""
+COLUMN_WIDTH = CW = 30
+MAX_TRACKS = 100  # Allows to load more tracks once.
 
-    COLUMN_WIDTH = CW = 30
-    MAX_TRACKS = 120  # Twice the number of tracks that fit in the viewport.
-
-    def interact(self):
-        """Controls the player through user interactions."""
-        with Player() as player:
-            while True:
-                print(
-                    "\nType: [play <track number>], [pause], [tracks], [more], [exit]"
-                )
-                command = input("> ").strip().lower()
-
-                if command.startswith("play"):
+def interact():
+    """Control the player through user interactions."""
+    with Player() as player:
+        while True:
+            print(
+                "\nType: play [<track number>] | pause | tracks | more | exit"
+            )
+            match input("> ").strip().lower().split():
+                case ["play"]:
+                    play(player)
+                case ["play", track]:
                     try:
-                        track_number = int(command.split()[1])
-                        self.play(player, track_number)
-                    except IndexError:  # Play first track.
-                        self.play(player)
+                        track_number = int(track)
+                        play(player, track_number)
                     except ValueError:
                         print("Please provide a valid track number.")
-                elif command == "pause":
-                    self.pause(player)
-                elif command == "tracks":
-                    self.tracks(player)
-                elif command == "more":
-                    # A higher number of tracks can't be clicked without scrolling.
-                    if len(player.tracklist.available_tracks) >= self.MAX_TRACKS:
-                        print("Can't load more tracks. Pick one from the track list.")
-                    else:
-                        player.tracklist.load_more()
-                        self.tracks(player)
-                elif command == "exit":
+                case ["pause"]:
+                    pause(player)
+                case ["tracks"]:
+                    display_tracks(player)
+                case ["more"] if len(player.tracklist.available_tracks) >= MAX_TRACKS:
+                    print("Can't load more tracks. Pick one from the track list.")
+                case ["more"]:
+                    player.tracklist.load_more()
+                    display_tracks(player)
+                case ["exit"]:
                     print("Exiting the player...")
                     break
-                else:
+                case _:
                     print("Unknown command. Try again.")
 
-    def play(self, player, track_number=None):
-        """Plays a track and shows info about the track."""
-        player.play(track_number)
-        print(player._current_track._get_track_info())
 
-    def pause(self, player):
-        """Pauses the current track."""
-        player.pause()
+def play(player, track_number=None):
+    """Play a track and shows info about the track."""
+    player.play(track_number)
+    print(player._current_track._get_track_info())
+
+def pause(player):
+    """Pause the current track."""
+    player.pause()
 
-    def tracks(self, player):
-        """Displays information about the currently playable tracks."""
-        header = (
-            f"{'#':<5} {'Album':<{self.CW}} {'Artist':<{self.CW}} {'Genre':<{self.CW}}"
-        )
-        print(header)
-        print("-" * 100)
-        for track_number, track in enumerate(
-            player.tracklist.available_tracks, start=1
-        ):
-            if track.text:
-                album, artist, *genre = track.text.split("\n")
-                album = self._truncate(album, self.CW)
-                artist = self._truncate(artist, self.CW)
-                genre = self._truncate(genre[0], self.CW) if genre else ""
-                print(
-                    f"{track_number:<5} {album:<{self.CW}} "
-                    f"{artist:<{self.CW}} {genre:<{self.CW}}"
-                )
+def display_tracks(player):
+    """Display information about the currently playable tracks."""
+    header = (
+        f"{'#':<5} {'Album':<{CW}} {'Artist':<{CW}} {'Genre':<{CW}}"
+    )
+    print(header)
+    print("-" * 100)
+    for track_number, track in enumerate(
+        player.tracklist.available_tracks, start=1
+    ):
+        if track.text:
+            album, artist, *genre = track.text.split("\n")
+            album = _truncate(album, CW)
+            artist = _truncate(artist, CW)
+            genre = _truncate(genre[0], CW) if genre else ""
+            print(
+                f"{track_number:<5} {album:<{CW}} "
+                f"{artist:<{CW}} {genre:<{CW}}"
+            )
 
-    @staticmethod
-    def _truncate(text, width):
-        """Truncates track information."""
-        return text[: width - 3] + "..." if len(text) > width else text
+def _truncate(text, width):
+    """Truncate track information."""
+    return text[: width - 3] + "..." if len(text) > width else text

python-selenium/src/bandcamp/web/element.py

@@ -4,19 +4,19 @@
 from selenium.webdriver.support import expected_conditions as EC
 
 from bandcamp.web.base import Track, WebComponent
-from bandcamp.web.locators import DiscoverPageLocatorMixin, TrackLocatorMixin
+from bandcamp.web.locators import DiscoverPageLocator, TrackLocator
 
 
-class TrackElement(WebComponent, TrackLocatorMixin):
-    """Models a playable track in Bandcamp's Discover section."""
+class TrackElement(WebComponent, TrackLocator):
+    """Model a playable track in Bandcamp's Discover section."""
 
     def play(self) -> None:
-        """Plays the track."""
+        """Play the track."""
         if not self.is_playing:
             self._get_play_button().click()
 
     def pause(self) -> None:
-        """Pauses the track."""
+        """Pause the track."""
         if self.is_playing:
             self._get_play_button().click()
 
@@ -25,7 +25,7 @@ def is_playing(self) -> bool:
         return "Pause" in self._get_play_button().get_attribute("aria-label")
 
     def _get_track_info(self) -> Track:
-        """Creates a representation of the track's relevant information."""
+        """Create a representation of the track's relevant information."""
         full_url = self._parent.find_element(*self.URL).get_attribute("href")
         # Cut off the referrer query parameter
         clean_url = full_url.split("?")[0] if full_url else ""
@@ -45,23 +45,23 @@ def _get_play_button(self):
         return self._parent.find_element(*self.PLAY_BUTTON)
 
 
-class DiscoverTrackList(WebComponent, DiscoverPageLocatorMixin, TrackLocatorMixin):
-    """Models the track list in Bandcamp's Discover section."""
+class DiscoverTrackList(WebComponent, DiscoverPageLocator, TrackLocator):
+    """Model the track list in Bandcamp's Discover section."""
 
     def __init__(self, parent: WebElement, driver: WebDriver = None) -> None:
         super().__init__(parent, driver)
         self.available_tracks = self._get_available_tracks()
 
     def load_more(self) -> None:
-        """Loads additional tracks in the Discover section."""
+        """Load additional tracks in the Discover section."""
         view_more_button = self._driver.find_element(*self.PAGINATION_BUTTON)
         view_more_button.click()
         # The button is disabled until all new tracks are loaded.
         self._wait.until(EC.element_to_be_clickable(self.PAGINATION_BUTTON))
         self.available_tracks = self._get_available_tracks()
 
     def _get_available_tracks(self) -> list:
-        """Finds all currently available tracks in the Discover section."""
+        """Find all currently available tracks in the Discover section."""
         self._wait.until(
             lambda driver: any(
                 e.is_displayed() and e.text.strip()

python-selenium/src/bandcamp/web/locators.py

@@ -1,7 +1,7 @@
 from selenium.webdriver.common.by import By
 
 
-class DiscoverPageLocatorMixin:
+class DiscoverPageLocator:
     DISCOVER_RESULTS = (By.CLASS_NAME, "results-grid")
     ITEM = (By.CLASS_NAME, "results-grid-item")
     PAGINATION_BUTTON = (By.ID, "view-more")
@@ -11,7 +11,7 @@ class DiscoverPageLocatorMixin:
     )
 
 
-class TrackLocatorMixin:
+class TrackLocator:
     PLAY_BUTTON = (By.CSS_SELECTOR, "button.play-pause-button")
     URL = (By.CSS_SELECTOR, "div.meta p a")
     ALBUM = (By.CSS_SELECTOR, "div.meta p a strong")

python-selenium/src/bandcamp/web/page.py

@@ -2,11 +2,11 @@
 
 from bandcamp.web.base import WebPage
 from bandcamp.web.element import DiscoverTrackList
-from bandcamp.web.locators import DiscoverPageLocatorMixin
+from bandcamp.web.locators import DiscoverPageLocator
 
 
-class DiscoverPage(WebPage, DiscoverPageLocatorMixin):
-    """Models the relevant parts of the Bandcamp Discover page."""
+class DiscoverPage(WebPage, DiscoverPageLocator):
+    """Model the relevant parts of the Bandcamp Discover page."""
 
     def __init__(self, driver: WebDriver) -> None:
         super().__init__(driver)
@@ -16,5 +16,5 @@ def __init__(self, driver: WebDriver) -> None:
         )
 
     def _accept_cookie_consent(self) -> None:
-        """Accepts the necessary cookie consent."""
+        """Accept the necessary cookie consent."""
         self._driver.find_element(*self.COOKIE_ACCEPT_NECESSARY).click()