Backend for Heat Map #75
Conversation
server/src/main/java/com/google/musicanalysis/site/YoutubeGenres.java
Outdated
Show resolved
Hide resolved
| @@ -96,9 +96,13 @@ protected void doGet(HttpServletRequest req, HttpServletResponse res) | |||
| JsonObject likedVideoRes; | |||
| JsonArray videos; | |||
| YoutubeGenres genreAnalysis = new YoutubeGenres(); | |||
| // needed to keep track of likedMusicHistory for heat map | |||
| int videosRetrieved = 0; | |||
There was a problem hiding this comment.
consider renaming to numVideosRetrieved, videosRetrieved sounds like a list of the retrieved videos
| @@ -31,9 +35,12 @@ public YoutubeGenres() { | |||
| * parses through youtube liked videos json array, | |||
| * updates hash map to contain frequency count of each music genre | |||
| * @param videos json array of youtube liked videos | |||
| * @param firstVideoCount the number of videos retrieved before this http call | |||
There was a problem hiding this comment.
consider renaming firstVideoCount to initialNumVideos. firstVideoCount sounds like it could be the index of the first video, or some count associated with the first video in the list
There was a problem hiding this comment.
+1. Probably something like "currentVideoIndex" will be clearest
| @@ -68,12 +75,17 @@ protected void calculateMusicCount(JsonArray videos) { | |||
| } | |||
| } | |||
|
|
|||
| if (isMusic && totalSubgenres == 0) { | |||
| if (isMusic) { | |||
| // likedMusicHistory records video numbers that are music | |||
There was a problem hiding this comment.
I don't think you need this comment, you already have a comment explaining what likedMusicHistory is when you first initialize it
| @@ -96,9 +96,13 @@ protected void doGet(HttpServletRequest req, HttpServletResponse res) | |||
| JsonObject likedVideoRes; | |||
| JsonArray videos; | |||
| YoutubeGenres genreAnalysis = new YoutubeGenres(); | |||
| // needed to keep track of likedMusicHistory for heat map | |||
There was a problem hiding this comment.
this comment doesn't really make sense to me, since it's not "keeping track" of likedMusicHistory but being used by it. likedMusicHistory also isn't part of this file, so that could come of as confusing to someone only looking at this file. consider rephrasing to something like "used to determine the position of a newly retrieved video in comparison to already retrieved videos while obtaining more videos in segments"
| @@ -111,7 +115,9 @@ protected void doGet(HttpServletRequest req, HttpServletResponse res) | |||
| } | |||
|
|
|||
| videos = likedVideoRes.getAsJsonArray("items"); | |||
| genreAnalysis.calculateMusicCount(videos); | |||
| // videosRetrieved keeps track of music video order in genreAnalysis | |||
There was a problem hiding this comment.
You don't need this comment since you already have a comment defining what this variable is when you initiated it
| @@ -31,9 +35,12 @@ public YoutubeGenres() { | |||
| * parses through youtube liked videos json array, | |||
| * updates hash map to contain frequency count of each music genre | |||
| * @param videos json array of youtube liked videos | |||
| * @param firstVideoCount the number of videos retrieved before this http call | |||
There was a problem hiding this comment.
+1. Probably something like "currentVideoIndex" will be clearest
| } | ||
| } | ||
| return; | ||
| return videosSize; |
There was a problem hiding this comment.
You can get this info from the parent function. No need to return it here, and that in fact obscures what this function is doing, since it analyzes the videos but also returns the total number. Someone interacting with this function will likely write:
totalVideos += calculateMusicVideos(videos, totalVideos);
This means 2 independent things are happening on one line that aren't strictly tied together. More clear is:
calculateMusicVideos(videos, totalVideos);
totalVideos += videos.size();
| videosRetrieved += genreAnalysis | ||
| .calculateMusicCount(videos, videosRetrieved); |
There was a problem hiding this comment.
See comment above. This should be split into 2 separate lines.
Implemented MVP backend necessary for heat map
likedMusicHistoryarray list needed for heat map