Skip to content

Added the rest of the array mutator functions along with events API#1

Closed
matthewmueller wants to merge 2 commits into
component:masterfrom
matthewmueller:master
Closed

Added the rest of the array mutator functions along with events API#1
matthewmueller wants to merge 2 commits into
component:masterfrom
matthewmueller:master

Conversation

@matthewmueller

Copy link
Copy Markdown
Member

Basically merged matthewmueller/array with this project. The additions follow the array API exactly.

I ran some basic performance tests (http://jsperf.com/mixinzssss/2) and it turns out, as you mentioned, wrapping is faster.

Feel free to merge it, take bits and pieces, or whatever. Let me know if you would like me to add or change anything.

Thanks!

@avetisk

avetisk commented Nov 12, 2012

Copy link
Copy Markdown

Adding events that's a pretty useful.

But isn't change event missing?

Would be pretty nice while using component/reactive.

@matthewmueller

Copy link
Copy Markdown
Member Author

Personally, I think change events should be done at the model level because it's not clear to me what the change event would be on a collection.

Maybe fired on both "add" and "remove", not sure...

@tj

tj commented Nov 12, 2012

Copy link
Copy Markdown
Member

cool looks good ill try and review soon. part of me wants to ditch the notion of a collection and use async enumerables to facilitate reactive "collections" instead. IMO the backbone way is kinda awkward, fetch things and stuff them in a collection that isn't async-friendly. I'll have to try out the other way and make sure it's flexible enough.

realistically you'd never do stuff like:

Items.all(function(items){
  items.select('complete').each(...)
})

you would write a method that queries with .complete only. It may be a little leaky api-wise to do
this because it would assume long-polling etc, but ideally you just iterate as you normally would,
and additional just roll in:

Items
.completed()
.each(function(item){
  reactive stuff here..
})

the only problem there is the whole removal process etc. I almost think we should inherit from Set instead of being an array-like container since it's really more of an ordered set.

Alternatively, using this collection stuff we could do the following, which might be better actually:

var items = Items.complete();
// async query blah blah "add" events

@matthewmueller

Copy link
Copy Markdown
Member Author

Yah, I like that a lot. I think the synchronization step could be broken out to allow support for long-polling, ws, localstorage, etc. It could make for a potentially useful low-level API as well.

A few questions around the example:

  • How would it handle adding and removing Items?
  • How would the API endpoint and methods (like completed) be determined? Pass the Item model into the enumerable, Items(Item)?

@matthewmueller

Copy link
Copy Markdown
Member Author

Any progress on this one? At the very least I'd like to have this library/enumerable emitting add and remove events.

@tj

tj commented Jan 10, 2013

Copy link
Copy Markdown
Member

haven't had a chance yet sorry! underlying events would be nice for flexibility, namely removal, but for the rest I think the async style enumerator would be a pretty nice thing

@matthewmueller

Copy link
Copy Markdown
Member Author

okay not a problem, i know you're busy :-D

One use-case that's been working pretty well for me:

var notes = new Collection;

notes.on('add', function(note) {
  var view = new NoteView(note),
  list.appendChild(view.show());
});

// Load notes from server
Note.all(function(err, collection) {
   for(var i = 0; i < collection.length; i++)
     notes.add(collection[i].model);
})

Allows you to bootstrap your app, and have it ready for any additions/removals later on.

@weepy

weepy commented Feb 6, 2013

Copy link
Copy Markdown

I would like to have this ^_^

It seems unecessary to emit the method name ?

@matthewmueller

Copy link
Copy Markdown
Member Author

yah the big ones are definitely add and remove, i haven't used any of the other method events yet, so we could probably remove them.

@weepy

weepy commented Feb 6, 2013

Copy link
Copy Markdown

you could argue it would be worth emitting something on 'sort' and
'reverse' since all the indexes are changed.

I think it would be nice to emit the index within the collection of the
element too ?

On Wed, Feb 6, 2013 at 4:49 PM, Matthew Mueller notifications@github.com
wrote:

yah the big ones are definitely add and remove, i haven't used any of the
other method events yet, so we could probably remove them.


Reply to this email directly or view it on GitHubhttps://github.com//pull/1#issuecomment-13191744
.

@domachine

Copy link
Copy Markdown

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants