I'm currently completing a project where I have to build a Spotify playlist creator. For part of this, I need to add or remove tracks from the playlist. I've coded a method that appears to work, but it is different from the official solution, so I just want to see whether there is a reason I shouldn't do it my way.

Specifically, they use .find and .filter methods where I have used .includes. Is there a downside to what I've done?

Their code

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

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

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

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

My code

addTrack(track) {
   let tracks = this.state.playlistTracks;
   if (!tracks.includes(track)) {
      tracks.push(track);
   }
   this.setState({playlistTracks: tracks});
}

removeTrack(track) {
   let tracks = this.state.playlistTracks;
   if (tracks.includes(track)) {
     let index = tracks.indexOf(track);
     tracks.splice(index, 1);
   }
   this.setState({playlistTracks: tracks});
}

CodePudding user response：

Yes, there is a significant difference, because includes() will only return true if you pass it the actual instance (by that I mean a reference that points to the same object) of track that you are looking for.

The provided solution compares tracks only based on the track ID, so that is something different.

See the following example:

const tracks = [
  {id: 1, title: "mysong1"},
  {id: 2, title: "mysong2"},
] 

function isTrackPresentIncludes(track){
  return tracks.includes(track);
}

function isTrackPresentFind(track){
  return tracks.find(it => it.id === track.id) !== undefined;
}


// this will be true
console.log("with includes(tracks[0]):\n", isTrackPresentIncludes(tracks[0]))
// this will be false as it's a different object
console.log("with includes({id: 1, title: \"mysong1\"}):\n", isTrackPresentIncludes({id: 1, title: "mysong1"}))

// this will be true
console.log("with find(tracks[0]):\n", isTrackPresentFind(tracks[0]))
// this will also be true
console.log("with find({id: 1, title: \"mysong1\"}):\n", isTrackPresentFind({id: 1, title: "mysong1"}))

You have the same issue with indexOf() in your removeTrack().

There is another thing I don't particularly like about the solution. find() returns the track that was found but that return value is never actually used so to my mind you should use some() instead which just returns true or false.

I don't think this is a problem here but it could potentially lead to unexpected behavior if an array would hold falsy values. Consider this:

const arrayWithFalsyValues = [
  0,  // zero is falsy!
  1, 
  2, 
  3, 
  4, 
  5, 
  6, 
  7, 
  8
] 

function isPresent(toBeFound){
  if(arrayWithFalsyValues.find(number => number === toBeFound)){
    console.log(`Value ${toBeFound} found in array`);
  }
  else{
    console.log(`Value ${toBeFound} NOT found in array`);
  }
}

console.log("Array:", arrayWithFalsyValues)
// this will work as expected
console.log("Contains 3?")
isPresent(3)
console.log("Contains 8?")
isPresent(8)
console.log("Contains 10?")
isPresent(10)
// now search for the falsy value -> incorrect result
console.log("Contains 0?")
isPresent(0)

CodePudding user response：

Issue is with referencing, You have make another reference of playlistTracks

addTrack(track) {
   let { playlistTracks } = this.state;
   let tracks = [...playlistTracks];
   if (!tracks.includes(track)) {
      tracks.push(track);
   }
   this.setState({ playlistTracks: tracks });
}

removeTrack(track) {
   let { playlistTracks } = this.state;
   let tracks = [...playlistTracks];
   if (tracks.includes(track)) {
     let index = tracks.indexOf(track);
     tracks.splice(index, 1);
   }
   this.setState({ playlistTracks: tracks });
}

MY SUGGESTION

addTrack(track) {
   const { playlistTracks } = this.state;
   const tracks = [...playlistTracks];
   const index = tracks.indexOf(track);
   if (index < 0) {
      tracks.push(track);
   }
   this.setState({ playlistTracks: tracks });
}

removeTrack(track) {
   const { playlistTracks } = this.state;
   const tracks = [...playlistTracks];
   const index = tracks.indexOf(track);
   if (index > -1) {
     tracks.splice(index, 1);
   }
   this.setState({ playlistTracks: tracks });
}