Project 1: Campus Life Starter Feedback

[spencm1]

Overall, good job! Your code is for the most part well-organized and easy to understand.

For this review, I'll provide code snippets and feedback on code snippets:

https://github.com/Kam227/site-unit2-project1-music-playlist-explorer-starter/blob/27da41a2decf815a5665cb4f0ea081267c1b28a5/music-playlist-creator/script.js#L1 You don't seem to use this variable and its name doesn't match its function, you can probably delete it.

https://github.com/Kam227/site-unit2-project1-music-playlist-explorer-starter/blob/27da41a2decf815a5665cb4f0ea081267c1b28a5/music-playlist-creator/script.js#L25-L26 Setting an id here is confusing, because this is a function that runs for multiple cards. Therefore, if we give them all the same id, this can lead to bugs in our code. Since your code doesn't need to keep track of each card at the id level, it's best not to include an id here IMO.

https://github.com/Kam227/site-unit2-project1-music-playlist-explorer-starter/blob/27da41a2decf815a5665cb4f0ea081267c1b28a5/music-playlist-creator/script.js#L63-L64 This is a clever workaround. You can get a clearer solution, however, with: heart.addEventListener('click', (event) => event.stopPropagation()); and then you could add the event listener as normal.

https://github.com/Kam227/site-unit2-project1-music-playlist-explorer-starter/blob/27da41a2decf815a5665cb4f0ea081267c1b28a5/music-playlist-creator/script.js#L106 Why is this line floating here?

https://github.com/Kam227/site-unit2-project1-music-playlist-explorer-starter/blob/27da41a2decf815a5665cb4f0ea081267c1b28a5/music-playlist-creator/script.js#L115 Is this line necessary?

https://github.com/Kam227/site-unit2-project1-music-playlist-explorer-starter/blob/27da41a2decf815a5665cb4f0ea081267c1b28a5/music-playlist-creator/script.js#L140-L148 Few comments on this:

• Why do we have a hardcoded 4 in our index swap?
• It's probably better to have this function take in an array parameter so it doesn't have to internally be aware of the playlist data, just for modularity.

https://github.com/Kam227/site-unit2-project1-music-playlist-explorer-starter/blob/27da41a2decf815a5665cb4f0ea081267c1b28a5/music-playlist-creator/script.js#L215 For organization purposes, I'd recommend splitting this into its own .js file

I would also recommend making smaller commit that are more descriptively named so that we can review/revert changes more easily

[ushanakg]

Spencer covered all the things I noticed but also wanted to add that in general, we try not to commit unnecessary comments (like the ones telling you to fill in footer content). Looks good Kam!