Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

how to remove an object from collection #26

Open
hungcaoduy opened this issue Feb 16, 2016 · 15 comments
Open

how to remove an object from collection #26

hungcaoduy opened this issue Feb 16, 2016 · 15 comments
Assignees
Labels

Comments

@hungcaoduy
Copy link

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

@choval
Copy link

choval commented Feb 16, 2016

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

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

@tsironis
Copy link
Owner

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

@hungcaoduy
Copy link
Author

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?

@tsironis
Copy link
Owner

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?

@hungcaoduy
Copy link
Author

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

@tsironis
Copy link
Owner

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.

@avgerin0s
Copy link
Collaborator

@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 avgerin0s self-assigned this Feb 21, 2016
@avgerin0s avgerin0s added the bug label Feb 21, 2016
@tsironis
Copy link
Owner

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

@tsironis
Copy link
Owner

ping @eavgerinos

@avgerin0s
Copy link
Collaborator

@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

@ai3gtmc
Copy link

ai3gtmc commented Aug 18, 2016

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

@tsironis
Copy link
Owner

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

@tsironis tsironis mentioned this issue Sep 7, 2016
@sandzOfTime
Copy link

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

@santiagochen
Copy link

any incoming good news for this issue?

@ngrecco
Copy link

ngrecco commented Sep 12, 2018

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)
    };

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants