Biaggio74 / jammming

CodeCademy project
0 stars 0 forks source link

Summary #1

Open farishkash opened 6 years ago

farishkash commented 6 years ago

Unfortunately there are too many errors and project itself isn't complete.

The biggest issues comes from your Spotify.js file as you aren't returning search data properly.

One example is this section of code starting with line 32

 search(term){
    this.getAccessToken();
    term = encodeURIComponent(term.trim());
    const url = `https://api.spotify.com/v1/search?type=track&q=${term}`;

    fetch(url, {

Why do you need to encode the uriComponent? You don't have to to do this.

Also this.getAccessToken() is not saved to the userAccessToken.

Even after I went through your Spotify.js and corrected it there was more problems. You didn't write your search function properly in your App.js file as it a promise and must wait for the data.

 search(term){
    let tracks = Spotify.search(term) 
    this.setState({searchResultTracks: tracks});
  }

This should be written as a promise. I feel like after going through this you add extra variables to save items and it adds leads to more mistakes. This part corrected below.


search(term) {
    Spotify.search(term).then(tracks => {
      this.setState({searchResults: tracks});
    });
  }

Even after I got this part working, the data wasn't being passed through as this project is incomplete.

jammming

I know we went through this on the advisor chat, the purpose here for the review is to point out potential issues, correct minor errors, and some better programming practices.

Unfortunately at this point it is turning into me rewriting the whole project so I am ending the review here. I know you are struggling and the bulk of the issues is in your Spotify.js but maybe go over the ajax request section in the course material to help you with this.

Biaggio74 commented 6 years ago

Hi Farish, thanks for the review. I'm sorry for the late reply but I was on a travel this whole week, plus I jumped into the API course. (bit confused between the two program) I start to debug the Jamming based on your suggestion, I think this is a good base to start. I also agree that the incomplete program gives too much work for you, which is not completely fair from my side. I try to bring it into a working level and be back if it is still something unclear.

Biaggio74 commented 6 years ago

Hi Farish, I went through all of the details in my code and finally got it work :) I pushed the new code to this repo. I also completed the steps for surge, but for the submission I factored back to this state.

I would say the biggest challenge was given by a "return" tag before a "fetch" method in Spotify.search()

If you could give me some hints why and where we should use return, that I would appreciate. In the SavePlayList() it works with return or without return just before the fetch. But in the serach() ...I spent the whole Saturday to debug one issue.

An other question of mine. Is it possible to chain more fetch method after each other? For instance the savePlayList() has more, and first I tried with it. So basically I chained one GET and two POST request after each other, with .then() But it seemed to be the variables are not passed from one request to another. So than I just keep them inside each other, which was ok.

I will try to re-submit the code through the Codecademy lesson as well.

Thank you.