Skip to content

Improve readability and testability of code#11

Open
joeyshi12 wants to merge 9 commits intoForrestKnight:masterfrom
joeyshi12:master
Open

Improve readability and testability of code#11
joeyshi12 wants to merge 9 commits intoForrestKnight:masterfrom
joeyshi12:master

Conversation

@joeyshi12
Copy link

@joeyshi12 joeyshi12 commented Nov 22, 2021

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 joeyshi12 marked this pull request as draft November 22, 2021 07:45
@joeyshi12 joeyshi12 marked this pull request as ready for review November 22, 2021 07:45
@joeyshi12 joeyshi12 changed the title Improve readability of code Improve readability and testability of code Nov 22, 2021
@ForrestKnight
Copy link
Owner

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.

@joeyshi12
Copy link
Author

joeyshi12 commented Dec 7, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants