3

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 } ]

enter image description here

Improve this question
5
  • Can you give a format for your json localdata or any fiddle link so that it should be more explanatory? Commented Jun 17, 2017 at 13:11
  • @breakit That's it added to the original question thanks! Commented Jun 17, 2017 at 14:34
  • What do you mean by "updating the key"? Commented Jun 17, 2017 at 14:41
  • @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) Commented Jun 17, 2017 at 14:43
  • @JohnthePainter please check my answer you have an additional error in your code. Commented Jun 22, 2017 at 20:28

5 Answers 5

Reset to default
4
+50

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.

Improve this answer
Sign up to request clarification or add additional context in comments.

12 Comments

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.
What do you mean by "remove"? It seems your code will always push a new item in the place of the one deleted.
Yes I have JUST worked that out haha! I don't want it to push the one that's been deleted, obviously.
I tried putting the push before the for loop but it always seems to return an empty array.
Another way to get this boolean is using filter() again: found = typefaces.filter(value => (value && value.id == typefaceID && name == "drf")).length > 0;
|
0

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.

Improve this answer

1 Comment

After doing this If you face any problem regarding localStorage ping me here.
0

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); } }
Improve this answer

Comments

-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'); } } } } });
Improve this answer

3 Comments

Thanks. Doesn't 'localStorage.removeItem()' only remove the entire key and not the splice from the key?
@JohnthePainter You will have to both, removing it from the array using splice and then removing that property from the local storage as well.
The splice makes sense but is localStorage.removeItem(removedTypeFace.name + 'Storage'); the correct thing to remove that property from local storage?
-1

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>

Improve this answer

2 Comments

Thanks for this! This doesn't seem to remove the item from the localStorage though? :(
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...

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.