Vanilla bars w/ BACKEND DATA!! #74
Conversation
client/js/bar.js
Outdated
| Object.values(DATA.genreData), | ||
| Object.keys(DATA.genreData), | ||
| DATA.maxGenreCount); |
There was a problem hiding this comment.
I'm not sure if Object.values and Object.keys is the best way to do it
I assume that the array returned from both methods should match up with each other
There was a problem hiding this comment.
If they match up then I think this is a clever-ish solution to your problem. You could also just change createBarChart to accept the entire map and then loop through the key-value pairs there. I personally prefer the second method, but I'd also like to see what @laptou has to say about it.
client/js/bar.js
Outdated
| Object.values(DATA.genreData), | ||
| Object.keys(DATA.genreData), | ||
| DATA.maxGenreCount); |
There was a problem hiding this comment.
If they match up then I think this is a clever-ish solution to your problem. You could also just change createBarChart to accept the entire map and then loop through the key-value pairs there. I personally prefer the second method, but I'd also like to see what @laptou has to say about it.
client/js/genre.js
Outdated
| * and displays on youtube-genre.html | ||
| */ | ||
|
|
||
| const genreBlock = document.getElementById('genres'); | ||
| document.body.onload = fetchMusicGenre(); |
There was a problem hiding this comment.
Why are you calling fetchMusicGenre() as an onload handler if you call it on line 24?
|
nit: there is no such thing as an ESLint module. these files are ES modules. ESLint is just a program that looks at your code and flags any stylistic errors. |
…nilla-bars-backend
gracetian6
force-pushed
the
vanilla-bars-backend
branch
from
6b99538
to
232e799
Compare
client/js/bar.js
Outdated
| @@ -59,4 +54,9 @@ function createBarChart(chartValues, chartCategories) { | |||
| } | |||
| } | |||
|
|
|||
| createBarChart(CHART_VALUES, CHART_CATEGORIES); | |||
| GENRE_ANALYSIS.then((genreAnalysisInfo) => { | |||
There was a problem hiding this comment.
I mentioned this in another review, but consider renaming GENRE_ANALYSIS to GENRE_ANALYSIS_TASK or GENRE_ANALYSIS_PROMISE so that it is extra clear that this object is a Promise.
| createBarChart(CHART_VALUES, CHART_CATEGORIES); | ||
| GENRE_ANALYSIS_PROMISE.then((genreAnalysisInfo) => { | ||
| createBarChart( | ||
| Object.values(genreAnalysisInfo.genreData), |
There was a problem hiding this comment.
My opinion here would be to pass the entire map rather than separating it out. In createBarChart, we start to lose the connection between these two arrays when you're accessing values in one array based on the length of the other array. Open to other thoughts though @geekster777?
There was a problem hiding this comment.
+1. It makes your function call more concise, and like Jess said, keeps the direct connection between your categories and their values.
| createBarChart(CHART_VALUES, CHART_CATEGORIES); | ||
| GENRE_ANALYSIS_PROMISE.then((genreAnalysisInfo) => { | ||
| createBarChart( | ||
| Object.values(genreAnalysisInfo.genreData), |
There was a problem hiding this comment.
+1. It makes your function call more concise, and like Jess said, keeps the direct connection between your categories and their values.
GENRE_ANALYSISingenre.jsand moved this JSON obj tobar.jsBackend data looks like:
FRONT END BAR CHART:
