Project Review - Githubissues

SidTheEngineer commented 5 years ago

Rubric Score

Criteria 1: App Creation, Folder Structure, and File Paths

Score Level: 4/4
Comment(s): The project folder structure looks perfect. I didn't see any issues with file paths and importing/exporting.

Criteria 2: Correct Implementation of User Flows (e.g. Searching, Token Generation, Save Playlist, etc.)

Score Level: 1/4
Comment(s): Right now I'm not able to test the actual functionality of the app due to the redirect URI not being white listed on Spotify's side (you can do this by logging in via developer.spotify.com)

Regarding Components: Overall great work setting up your components, it looks like you've got most of the app set up appropriately. There are a few areas for improvement that I noticed that can clean up the code some:

addTrack()/removeTrack() - In our App.js our add and remove track methods are a little bit more complicated than they need to be. For removeTrack, we can simply create a new playlistTracks array that we will set the new state to, using the filter method to remove the track that's passed in:

removeTrack(track) {
  let tracks = this.state.playlistTracks;
  tracks = tracks.filter(currentTrack => currentTrack.id !== track.id);

  this.setState({playlistTracks: tracks});
}

For addTrack, we won't need to create a new function inside of the method. We can look for if the track being added has already been added, and if not, we push the track to playlistTracks array and update state accordingly:

addTrack(track) {
  let tracks = this.state.playlistTracks;
  if (tracks.find(savedTrack => savedTrack.id === track.id)) {
    return;
  }

  tracks.push(track);
  this.setState({playlistTracks: tracks});
}

Additionally, nice work adding the functionality to keeping duplicated tracks out of the newly created playlist!

Criteria 3: Proper JavaScript and JSX Syntax and Formatting

Score Level: 3/4
Comment(s): There are a few areas where we can adjust indentation throughout the app (check out SearchBar.js and TrackList.js). Beyond this, I would recommend removing extra console.log's throughout so as to clean up some of the files.

Criteria 4: Implementation Adheres to DRY Principles

Score Level: 4/4
Comment(s): I didn't really see too much repeated or unnecessary code throughout the project. Nice work keeping things DRY.

Overall Score: 12/16

Overall, the project is set up appropriately and looks solid! A lot of the areas for improvement I noticed had mostly to do with shortening down some of the methods and cleaning up formatting of the code. Outside of this, we'll want to make sure we add the redirect URI to the white list in your Spotify developer account so as to avoid the INVALID_CLIENT: Invalid redirect URI error.

PatLeroux commented 5 years ago

Overall i pass the test right?

PatLeroux commented 5 years ago

Done:

Comment(s): Right now I'm not able to test the actual functionality of the app due to the redirect URI not being white listed on Spotify's side (you can do this by logging in via developer.spotify.com)

PatLeroux / jammming