yt-dislikes icon indicating copy to clipboard operation
yt-dislikes copied to clipboard

Improve readability and testability of code

Open joeyshi12 opened this issue 4 years ago • 2 comments

Issue

Having all the logic bundled within a single main method is difficult to understand and test, so I broke up the code as much as I could in a way that could address both these problems.

Changes

  • Code hygiene:
    • Added wrapper class (YoutubeClientAdapter) to interface functions from the youtube api client and implement helper methods for the main method
      • If this approach is accepted, we can mock the api client using mockito and start writing YoutubeClientAdapter tests
    • Added placeholders (views, likes, etc..) to the statistics comment (stat_comment) to simplify generating new comment strings using the String.format method (perhaps we should think about localizing this string in the future?)
  • Other improvements:
    • Get video statistics in one batch fetch in YoutubeClientAdapter#get_channel_videos rather than invoking a fetch operation for each video
    • Set ratio to 0 in main method if there are 0 likes to avoid division by 0 error
    • Added new requirement API_KEY for the .env file
      • Note: I was not able to get a successful response from the comment insert/update api operations without this
    • Added __pycache__, .vscode to .gitignore
Screen Shot 2021-11-23 at 12 44 22 AM

joeyshi12 avatar Nov 22 '21 07:11 joeyshi12

A lot of this is very helpful! I haven't gone through all of it, but I will. The only issue I see with it is that it's quite a bit different from the YouTube Data API doc code. That may be better, but it could cause some trouble in the future for others trying to understand the code. Let me think on this a bit longer.

ForrestKnight avatar Dec 07 '21 18:12 ForrestKnight

A lot of this is very helpful! I haven't gone through all of it, but I will. The only issue I see with it is that it's quite a bit different from the YouTube Data API doc code. That may be better, but it could cause some trouble in the future for others trying to understand the code. Let me think on this a bit longer.

Good point. I can isolate some of the YouTube boilerplate code better and remove some of my changes to that specific part later. But, I think using an adapter for the api client helps to unbundle some complex logic in the main method and it will also offer some nice auto-completion hints from the ide (good for potential contributors) since we would have an interface for the api client in the codebase.

joeyshi12 avatar Dec 07 '21 20:12 joeyshi12