lockr icon indicating copy to clipboard operation
lockr copied to clipboard

how to remove an object from collection

Open hungcaoduy opened this issue 8 years ago • 15 comments

I added my product into shopping cart by using:

Lockr.sadd("cart", {id: 123, name: abc}); Lockr.sadd("cart", {id: 456, name: def});

How to remove a particular object from cart? e.g I later want to remove the first object {id: 123, name: abc}

thanks

hungcaoduy avatar Feb 16 '16 09:02 hungcaoduy

var carts = Lockr.smembers("cart");
carts.splice(0,1);  
Lockr.set("cart",carts);

a nice Lockr.srempos("cart",0) would be nice...

choval avatar Feb 16 '16 18:02 choval

Cool idea! Let's name it Lockr.srempos, or maybe Lockr.sremat which I think is more concise. What do you think?

tsironis avatar Feb 16 '16 23:02 tsironis

Lockr.srempos or Lockr.sremat would be nice!

If you can go further, how about something like Lockr.srem("cart", {id: 123}) ? an optional option object may help to search and remove matched item/items from a collection. does that make sense?

hungcaoduy avatar Feb 17 '16 08:02 hungcaoduy

Just clarifying Lockr.srem works like _.without though, so if you provide a JSON object, srem will remove all instances containing that key-value pair. Is that the behaviour you would expect?

tsironis avatar Feb 17 '16 09:02 tsironis

I understand the example when the item to add is primitive data like below: Lockr.sadd("wat", 1); Lockr.sadd("wat", 2); [1, 2] Lockr.srem("wat", 1); //[2]

How to do if the Lockr.sadd does not work on primitive data as 1, 2 but object like {id: 123, name: abc}? At a state when Lockr.smembers('cart') get [{id: 123, name: abc}, {id: 456, name: def}], I expect that Lockr.srem('cart', {id: 123}) would remove the first object and Lockr.smembers('cart') would result [{id: 456, name: def}] but it does not work

hungcaoduy avatar Feb 17 '16 10:02 hungcaoduy

Indeed, it doesn't work currently for objects like this yet. I was referring to the existing functionality of srem that removes all instances of a value in a specified array. The proposed changes will have to work in a similar manner, removing all objects that contain instances of key-value pair in specified array.

tsironis avatar Feb 17 '16 15:02 tsironis

@hungcaoduy @tsironis True, I confirm this as a bug.

The reason behind this is that indexOf, used by sismember which in turn is used by srem doesn't work on array of Objects.

I'll craft a patch tomorrow as well as add some regression tests.

Thanks for reporting!

PS: As for srempos, I'd prefer to not break the Redis-like API.

avgerin0s avatar Feb 21 '16 01:02 avgerin0s

@eavgerinos what we would be a Redis-like method name for srempos? Is there an equivalent?

tsironis avatar Feb 25 '16 09:02 tsironis

ping @eavgerinos

tsironis avatar Mar 16 '16 09:03 tsironis

@tsironis http://redis.io/commands#set nope AFAIK.

IMHO, we should fix the behavior of srem in case of objects instead of adding an srempos

avgerin0s avatar Mar 16 '16 11:03 avgerin0s

I just ran into this dilemma myself. Anyone ever got it fixed?

ai3gtmc avatar Aug 18 '16 03:08 ai3gtmc

This is not fixed yet. Temporary solution is the one mentioned by @choval though.

tsironis avatar Aug 26 '16 12:08 tsironis

I take it this issue hasn't been resolved as yet.

sandzOfTime avatar Jan 01 '18 18:01 sandzOfTime

any incoming good news for this issue?

santiagochen avatar Apr 23 '18 08:04 santiagochen

Hello everyone. Here's a working implementation from srempos. You can use the index that matches the smembers array.

Lockr.srem = function(key, value, options, positionToRemove) 
{
    var query_key = this._getPrefixedKey(key, options),  json,index;
    var values = Lockr.smembers(key, value);

    if (positionToRemove !== undefined)
    {
        index = positionToRemove;
    }
    else index = values.indexOf(value);

    if ((index > -1) && (index < values.length))
      values.splice(index, 1);

    json = JSON.stringify({"data": values});

    try {
      localStorage.setItem(query_key, json);
    } catch (e) {
      if (console) console.warn("Lockr couldn't remove the "+ value +" from the set "+ key);
    }
  };

    Lockr.srempos = function(key, pos) {
      return Lockr.srem(key, undefined, undefined, pos)
    };

ngrecco avatar Sep 12 '18 15:09 ngrecco