Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backend for Heat Map #75

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
4 changes: 2 additions & 2 deletions client/js/genre.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@
* and displays on youtube-genre.html
*/

const genreBlock = document.getElementById('genres');

/**
* fetches genre count hashmap from /api/youtube and updates html
*/
async function displayMusicGenre() {
const genreBlock = document.getElementById('genres');

// keep track of num_videos in URL w/o reload
history.pushState('', '', `youtube-genre.html`);
gracetian6 marked this conversation as resolved.
Show resolved Hide resolved

Expand All @@ -21,4 +22,3 @@ async function displayMusicGenre() {
const genreCount = await response.text();
genreBlock.innerHTML = genreCount;
}

1 change: 1 addition & 0 deletions client/youtube-genre.html
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ <h1>What music do you listen to on youtube?</h1>
<input type="submit" onclick="displayMusicGenre()"/>
</p>
</div>
<div id='genres'></div>
</div>
<main>
<div class='hero'>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import com.google.gson.JsonObject;
import com.google.gson.JsonArray;
import com.google.gson.Gson;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.Map;
import java.lang.Math;
Expand All @@ -22,6 +23,9 @@ public class YoutubeGenres {
public HashMap<String, Integer> genreData = new HashMap<String, Integer>();
public int totalMusic = 0;
public int maxGenreCount = 0;
// element of value x means xth latest video is music
// needed for heat map
public ArrayList<Integer> likedMusicHistory = new ArrayList<Integer>();

public YoutubeGenres() {

Expand All @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1. Probably something like "currentVideoIndex" will be clearest

* @return number of videos retrieved in this call
*/
protected void calculateMusicCount(JsonArray videos) {
for (int i = 0; i < videos.size(); i++) {
protected int calculateMusicCount(JsonArray videos, int firstVideoCount) {
int videosSize = videos.size();
for (int i = 0; i < videosSize; i++) {
JsonObject video = videos.get(i).getAsJsonObject();
JsonObject topicDetails = video.getAsJsonObject("topicDetails");

Expand Down Expand Up @@ -68,12 +75,17 @@ protected void calculateMusicCount(JsonArray videos) {
}
}

if (isMusic && totalSubgenres == 0) {
if (isMusic) {
// likedMusicHistory records video numbers that are music
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need this comment, you already have a comment explaining what likedMusicHistory is when you first initialize it

likedMusicHistory.add(firstVideoCount + i);

if (totalSubgenres == 0) {
// video only classified as Music so we update as "Other music"
this.updateGenre("Other music");
this.updateGenre("Other music");
}
}
}
return;
return videosSize;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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();

}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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"

int videosRetrieved = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider renaming to numVideosRetrieved, videosRetrieved sounds like a list of the retrieved videos


// next Page Token must be an empty string for first http call
String nextPageToken = "";
// Make multiple paginated calls to youtube API.
// Each call has a new page token
while (nextPageToken != null) {
youtubeResBody = getYoutubeRes(API_KEY,
accessToken.toString(),
Expand All @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need this comment since you already have a comment defining what this variable is when you initiated it

videosRetrieved += genreAnalysis
.calculateMusicCount(videos, videosRetrieved);
Comment on lines +119 to +120
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment above. This should be split into 2 separate lines.


nextPageToken = getNextPageToken(likedVideoRes);
}
Expand Down