Removing specific item from localStorage

Question

I'm adding some items to localStorage, using jQuery/JS, which is all fine but in trying to remove a specific item within the array (if it's the same item) is proving difficult.

In my console logs (for testing) it seems to clear the [Object] but it's not updating the key. Perhaps my hierarchy is wrong... any ideas?

//
function addToStorage(elem, name) {

    localData = localStorage.getItem(name + 'Storage');
    var typefaces;

    if (localData == 'null' || !localData) {
        typefaces = [];
    } else {
       typefaces = JSON.parse(localData);
    }

    typefaceID = elem.find('input').val();
    typefaceName = elem.find('input').attr('data-name');
    typefacePrice = elem.find('input').attr('data-price');
    typefaceQty = 1;

    $.each(typefaces, function(index, value) {
        if (value !== null) {
            if (value.id == typefaceID) {
                if (name == 'drf') {
                    //console.log(index);
                    //console.log(typefaces);
                    typefaces.splice(index, 1);
                    //console.log(typefaces);
                }
            }
        }
    });

    typefaces.push({
        'id': typefaceID,
        'name': typefaceName,
        'price': typefacePrice,
        'qty': typefaceQty
    });

    localStorage.setItem(name + 'Storage', JSON.stringify(typefaces));

}

//
$(document).on('click', 'summary.cart td.font', function(e) {
    e.preventDefault();
    addTo = $(this);
    addTo.each(function() {
        addToStorage(addTo, 'drf');
    });
});

This is an example of the localData once it's been added to.

[  
   {  
      "id":"dr-raymond-desktop-40374",
      "name":"DR-Raymond",
      "format":"Desktop (OTF)",
      "price":"15.00",
      "qty":1
   },
   {  
      "id":"dr-raymond-webfont-39949",
      "name":"DR-Raymond",
      "format":"Webfont (WOFF)",
      "price":"15.00",
      "qty":1
   }
]

Can you give a format for your json localdata or any fiddle link so that it should be more explanatory? — breakit, Jun 17 '17 at 13:11
@XavierJ Sorry if you see my image attachment from Chrome DevTools when inspecting the localStorage. The 'key' in this instance is the localStorage and the 'value' is the contents (localData array) — John the Painter, Jun 17 '17 at 14:43
@JohnthePainter please check my answer you have an additional error in your code. — , Jun 22 '17 at 20:28

alonkol · Accepted Answer · 2017-06-18T14:53:11.973

4

Never add/remove elements from an array while iterating over it using "foreach". Instead, try this:

for (index = typefaces.length - 1; index >= 0; index--){
    value = typefaces[index];
    if (value !== null) {
        if (value.id == typefaceID) {
            if (name == 'drf') {
                typefaces.splice(index, 1);
            }
        }
    }
});

Another way to do this more elegantly is by using ES6 filter():

typefaces = typefaces.filter(value => !(value && value.id == typefaceID && name == "drf"));

Also, you are comparing localData to the literal string 'null' which is kind of pointless. Your second condition - if (!localData) is enough in this case, and will handle it properly.

edited Jun 18 '17 at 14:53

answered Jun 17 '17 at 15:13

alonkol

380
2
8

Thanks for this. It's still not removing it from the localStorage. If I click it once it adds it to the localStorage but if I click it again, to remove, it doesn't. – John the Painter Jun 17 '17 at 16:43
2

What do you mean by "remove"? It seems your code will always push a new item in the place of the one deleted. – alonkol Jun 18 '17 at 14:58
Yes I have JUST worked that out haha! I don't want it to push the one that's been deleted, obviously. – John the Painter Jun 18 '17 at 15:16
I tried putting the push before the for loop but it always seems to return an empty array. – John the Painter Jun 18 '17 at 15:34
It seems to me you are trying to acheive somekind of "toggle" behavior. You named this function "addToStorage". Are you trying to do something other than adding an element to the storage? – alonkol Jun 19 '17 at 17:45
Yes, as described in the original question, it is a kind of 'toggle'. The 'addToStorage' is perhaps slightly badly named but when they click the item to add... if they click the item that is already added then yes it should remove it from the localStorage. – John the Painter Jun 19 '17 at 17:50
I think that's happening with mine is that it's checking the items then pushing it... whereas it should push it then check it (but then it would never add it) so there needs to be a way that it checks if it's in the array... if it is then it removes it otherwise push it to array. – John the Painter Jun 19 '17 at 18:22
You can simply use the for-loop implementation I suggested above then. Initialize a boolean to `found = false;` before the `for` loop, and assign `found = true;` instead of splicing it. Now you have a boolean which indicates if this element is already in the array. Use it in an `if` statement to acheive what you're looking for – alonkol Jun 19 '17 at 18:29
1

Another way to get this boolean is using filter() again: `found = typefaces.filter(value => (value && value.id == typefaceID && name == "drf")).length > 0`; – alonkol Jun 19 '17 at 18:31
Thanks for all the help. Shouldn't I splice it if it does exist, though? – John the Painter Jun 19 '17 at 19:19
if (found) { splice it } else { push it } – alonkol Jun 20 '17 at 10:51
I gave you the 50 points and accepted answer as you mentioned the if found implementation which after some testing this was the way that worked! Thank you and thanks everyone. – John the Painter Jun 23 '17 at 10:24

score 0 · Answer 2 · answered Jun 21 '17 at 10:47

The problem lies in your splice method usage. Note that, according to MDN splice modifies the array in place and returns a new array containing the elements that have been removed. So when using a loop and trying to remove the some elements, splice will make shifting of elements. This is because iterating incrementally through the array, when you splice it, the array is modified in place, so the items are "shifted" and you end up skipping the iteration of some. Looping backwards fixes this because you're not looping in the direction you're splicing.

Solution 1

Loop backwards while splicing.

for(var i = typefaces.length; i--;)
{
            if (typefaces[i] !== null) 
            {

                if (typefaces[i] == typefaceID) 
                {
                        typefaces.splice(i, 1);
                }
            }
 }

Working bin link here.

Solution 2

Its usually faster to generate a new array instead of modifying the existing one. So, your code will look like

ty = [];
   $.each(typefaces, function(index, value) {
        if (value !== null) {

            if (value.id != typefaceID) {
                    ty.push(value);
            }
        }
    });
  typefaces = ty;

Working bin link is here. After that there is no problem found in getting and setting your localStorage.

After doing this If you face any problem regarding localStorage ping me here. — breakit, Jun 21 '17 at 10:56

score 0 · Answer 3 · answered Jun 22 '17 at 20:28

You have an additional error in your code, you write

if (value !== null) {
    if (value.id == typefaceID) {
        // name will always have the value drf 
        // since you pass it in your function
        // when you call it addToStorage(addTo, 'drf');
        if (name == 'drf') {
            typefaces.splice(index, 1);
        }
    }
}

when it should be

if (value !== null) {
    if (value.id == typefaceID) {
        // here the comparison should be between value.name and name
        if (value.name == name) {
            typefaces.splice(index, 1);
        }
    }
}

which can be also written

if (value !== null) {
    if (value.id == typefaceID && value.name == name) {
            typefaces.splice(index, 1);
    }
}

score -1 · Answer 4 · answered Jun 07 '17 at 16:52

-1

You are mutating the array by splicing it. The only localstorage items that are being re written are the ones that are left in the array.

So delete the item from the storage when you splice an object from the array.

$.each(typefaces, function(index, value) {
        if (value !== null) {
            if (value.id == typefaceID) {
                if (name == 'drf') {
                    //console.log(index);
                    //console.log(typefaces);
                    var removedTypeFace = typefaces.splice(index, 1);
                    //console.log(typefaces);

                    if(removedTypeFace) {
                        localStorage.removeItem(removedTypeFace.name + 'Storage');
                    }
                }
            }
        }
    });

answered Jun 07 '17 at 16:52

Sushanth --

55,259
9
66
105

1

Thanks. Doesn't 'localStorage.removeItem()' only remove the entire key and not the splice from the key? – John the Painter Jun 07 '17 at 16:57
@JohnthePainter You will have to both, removing it from the array using `splice` and then removing that property from the local storage as well. – Sushanth -- Jun 08 '17 at 21:33
1

The splice makes sense but is `localStorage.removeItem(removedTypeFace.name + 'Storage');` the correct thing to remove that property from local storage? – John the Painter Jun 09 '17 at 10:38

score -1 · Answer 5 · answered Jun 17 '17 at 15:19

OK. You're going to kick yourself. The first time you pull from localStorage, the getItem method DOES return a null value, but you're trying to compare it to a string ('null', in quotes). Fail. I changed localData == 'null' to localData == null.

The second part of the expression tries to see the value returned from getItem is actually defined, which is good. I changed that to be more explicit, by way of typeof.

Explicit variable declarations help, too.

function addToStorage(elem, name) {

    var localData = localStorage.getItem(name + 'Storage');
    var typefaces;

    if (localData == null || typeof(localData) == 'undefined') {
        typefaces = [];
    } else {
       typefaces = JSON.parse(localData);
    }

    var typefaceID = elem.find('input').val();
    var typefaceName = elem.find('input').attr('data-name');
    var typefacePrice = elem.find('input').attr('data-price');
    var typefaceQty = 1;

    $.each(typefaces, function(index, value) {
        if (value !== null) {
            if (value.id == typefaceID) {
                if (name == 'drf') {
                    //console.log(index);
                    //console.log(typefaces);
                    typefaces.splice(index, 1);
                    //console.log(typefaces);
                }
            }
        }
    });

    typefaces.push({
        'id': typefaceID,
        'name': typefaceName,
        'price': typefacePrice,
        'qty': typefaceQty
    });

    localStorage.setItem(name + 'Storage', JSON.stringify(typefaces));

}

//
jQuery(document).on('click', 'summary.cart td.font', function(e) {
    e.preventDefault();
    addTo = $(this);
    addTo.each(function() {
        addToStorage(addTo, 'drf');
    });
});

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<summary class='cart'><table><tr>
<td class='font' >Click Here for Item 1
<input data-name='abc' data-price='2.00' value="Item 1" /></td></tr>
<tr>
<td class='font' >Click Here for Item 2
<input data-name='def' data-price='4.00' value="Item 2" /></td></tr></table>
</summary>

Thanks for this! This doesn't seem to remove the item from the localStorage though? :( — John the Painter, Jun 17 '17 at 17:47
It seems to remove it fine from the array (if I console.log the array before and after the removal it shows it changing from [Object] to [] BUT in it still seems to exist in the localStorage). Maybe my line `localStorage.setItem(name + 'Storage', JSON.stringify(typefaces));` isn't doing what I think it is... — John the Painter, Jun 17 '17 at 17:55

Removing specific item from localStorage

5 Answers5

Solution 1

Solution 2

Linked