search

marcuswestin / store.js

Cross-browser storage for all use cases, used across the web.
MIT License
14.02k stars 1.33k forks source link

suggestion for forEach #144

Closed blq closed 7 years ago

blq commented 8 years ago

I think it would be useful if forEach by default iterated backwards instead. Then, similar to when iterating a standard JavaScript array, the callback could remove items (the current) without invalidating the loop. Could also add elements. Since the low-level index is never exposed to the callback a user wouldn't/shouldn't notice this change. Hope I'm not missing something in the standard here, seem to work in my quick tests at least.

Regards /Fredrik Blomqvist

marcuswestin commented 7 years ago

Sure! Though it probably should have a new name since changing forEach could break current code.

blq commented 7 years ago

Ok, though I'd say you have to make an effort to fail, code has to be written assuming a same search-match hits a specific object first..

Feel free to have a look at my take on store.js (started as fork, now a full rewrite, and drops IE polyfill) https://github.com/blq/blq/blob/master/store.js (Also does some other stuff like iterators that we could discuss in another thread maybe)

/Fredrik Blomqvist

marcuswestin commented 7 years ago

I've wanted to rehaul store.js for a while, and make it trivial to create plugins with additional functionality. Maybe it's time... :) On Wed, Dec 14, 2016 at 11:35 AM Fredrik Blomqvist notifications@github.com wrote:

Ok, though I'd say you have to make an effort to fail, code has to be written assuming a same search-match hits a specific object first..

Feel free to have a look at my take on store.js (started as fork, now a full rewrite, and drops IE polyfill) https://github.com/blq/blq/blob/master/store.js (Also does some other stuff like iterators that we could discuss in another thread maybe)

/Fredrik Blomqvist

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/marcuswestin/store.js/issues/144#issuecomment-267083146, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIDfys2j9pu_pQYnYOI0sbOei7tNsDAks5rIBrbgaJpZM4G0qcP .

-- -- while mobile

marcuswestin commented 7 years ago

Version 2 has now been built!! It has an each function which behaves as you have suggested. See v2 branch and stay tuned for release in the new year.

blq commented 7 years ago

great! but, I can't find the change in v2? have you pushed it publicly yet?

marcuswestin commented 7 years ago

It's in the v2 branch. Here are some relevant links:

https://github.com/marcuswestin/store.js/tree/v2-dev https://github.com/marcuswestin/store.js/blob/v2-dev/store-engine.js#L36 https://github.com/marcuswestin/store.js/tree/v2-dev/storage

On Sun, Jan 1, 2017 at 4:20 PM Fredrik Blomqvist notifications@github.com wrote:

great! but, I can't find the change in v2? have you pushed it publicly yet?

— You are receiving this because you modified the open/close state.

Reply to this email directly, view it on GitHub https://github.com/marcuswestin/store.js/issues/144#issuecomment-269920136, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIDf3EEGvDhYe_1N_iRAb4aEgCkWxb3ks5rOBiUgaJpZM4G0qcP .

-- -- while mobile

blq commented 7 years ago

Ok, now I see, blame it on the file-reorganization ;) One issue I find though is that memoryStorage will not behave identical to the other each(). You need to take a copy of the keys before looping to not risk visiting elements added during the loop.

marcuswestin commented 7 years ago

Good catch! I'll add a test to make sure all storages do it the way we expect.

On Mon, Jan 2, 2017 at 3:42 AM Fredrik Blomqvist notifications@github.com wrote:

Ok, now I see, blame it on the file-reorganization ;)

One issue I find though is that memoryStorage will not behave identical to the other each().

You need to take a copy of the keys before looping to not risk visiting elements added during the loop.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/marcuswestin/store.js/issues/144#issuecomment-269945306, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIDf0FgIxFzKyV20TLt8Uu7pVtWIchwks5rOLh0gaJpZM4G0qcP .

-- -- while mobile