CODEWITHSUNDEEP
javascriptreactjs
Thinker
Thinker

Reputation: 5356

react push unique objects in array

I have a function that is supposed to add unique elements to an array:

addLanguage = (val) => {

    for(let i=0; i< val.length; i++){
        console.log(val[i].id)
        if(this.state.createNewDeliveryData.languages.indexOf(val[i].id === -1)){
             this.state.createNewDeliveryData.languages.push(val[i])
        }
    }
}

the argument 'val' each time gets an array with appended elements (I have a multi select box on UI from where I add languages), that's why in If loop I am trying to check if my languages array already has that specific element. However this has no effect and at the end my languages array contains same elements i.e. same element is added twice or thrice etc. on subsequent calls.

['German']
['German', 'German', 'Arabic']
...

What am I doing wrong?

Upvotes: 2

Views: 6988

Answers (3)

Chris
Chris

Reputation: 59511

You have a few issues with your code:

  • First of all, never, ever mutate this.state directly. It is anti-pattern and may yield undesired results. Use this.setState() instead.

  • There is no need to do indexOf in a loop. indexOf will iterate the array and return the index of the searched item, or -1 if it was not in the array.

  • You have to create a copy for the this.state.createNewDeliveryData object and for the languages array.

That said, one solution could be:

addLanguage = (val) => {
  if(this.state.createNewDeliveryData.languages.indexOf(val) === -1) {
    let obj = Object.assign({}, this.state.createNewDeliveryData);  //shallow-copy object
    let arr = obj.languages.slice();  //copy array
    arr.push(val);  //push the value
    obj.languages = arr;  //assign the new array to our copied object
    this.setState({createNewDeliveryData: obj});  //replace the old object with the new
  }
}

or in a more compact form:

addLanguage = (val) => {
  if(this.state.createNewDeliveryData.languages.indexOf(val) === -1) {
    let obj = Object.assign({}, this.state.createNewDeliveryData);  //shallow-copy object
    obj.languages = obj.languages.slice().push(val);
    this.setState({createNewDeliveryData: obj});  //replace the old object with the new
  }
}

Upvotes: 2

Christopher Chiche
Christopher Chiche

Reputation: 15335

You are looking for the index of false in your array. That's because, indexOf(val[i].id === -1) will always check indexOf(false) which is -1. So you re-add each language.

You are probably after an algorithm more similar to this: 

addLanguage = (val) => {
    // Get a copy of the languages state. 
    const languages = [...this.state.createNewDeliveryData.languages];
    for(let i=0; i< val.length; i++){
        if(languages.indexOf(val[i].id) === -1) { // notice that there is a parenthesis after `id`.
            languages.push(val[i].id)
        }
    }
    this.setState({createNewDeliveryData: {languages}}); // update state and trigger re-render if there are new languages. 
}

Note that you should always use setState to mutate the state in react.

Upvotes: 3

kind user
kind user

Reputation: 41893

Instead of checking each element in a loop, you could just use Set object to remove every duplicated element.

Note: I assume that the val variable is an array.

addLanguage = (val) => [...new Set(val)];

Upvotes: 2

Related Questions