brianmhunt / knockout-fast-foreach

Foreach. Faster. For Knockout.
MIT License
59 stars 18 forks source link

Add `$isFirst`, `$isLast` #24

Open brianmhunt opened 9 years ago

brianmhunt commented 9 years ago

These can theoretically all be O(1).

cervengoc commented 9 years ago

What exactly would $atIndex mean?

brianmhunt commented 9 years ago

@cervengoc A truthy/falsy test e.g. $atIndex(4) returns true if the item is at index 4. I had it in my mind that this could be O(1), but I'm not entirely convinced I'm right. :grin:

cervengoc commented 9 years ago

Well I don't know. At first glance it seems to me that any solution which depends on individual index values other than first or last leads to unavoidable index maintenance in one way or another. And on the other hand, personally I don't like the $atIndex function, I think it's not usable too well and a bit counter-intuitive (one would expect it to be an observable like all others). But that's really kind of a personal preference.

Anyway, please see my comment at the original topic too.

brianmhunt commented 9 years ago

@cervengoc Yeah, I'm with you. I think it'll just be $isFirst and $isLast

AdamWillden commented 9 years ago

I have just come across a need for isLast but also an isNotLast. Is it worth adding:

They should all still be O(1) ...or not?

brianmhunt commented 9 years ago

@AdamWillden They should all be O(1); the isNotLast & isNotFirst are IMHO expressed better as !$isLast and !$isFirst respectively. I.e. there should be no need for property values for these as they can be computed on the fly. At least, that's my thinking. :smile: What do you think?

IanYates commented 9 years ago

Possibly coming from ignorance, but are $isLast and $isFirst observables or values? If they're observables then !$isLast wouldn't work but !$isLast() would. I see this as being a cousin of KO including both if and ifnot bindings. Is there any drawback to having to evaluate !isLast()

brianmhunt commented 9 years ago

Thanks @IanYates – Yes, !isLast() is the right syntax. :) It should be an observable (though may in the implementation end up being an opaque defineProperty).

!isLast() ends up being an "implied computed', just as isNotLast would likely be– though with isNotLast there's an extra layer.

I'm not totally averse to adding isNotLast/First, but I don't want to clutter up the API with things that are better written with a simple negation operator.

AdamWillden commented 9 years ago

No problem @brianmhunt. Just floating the idea but what you say makes sense. I think it'd be nicer as an opaque defineProperty

brianmhunt commented 9 years ago

@AdamWillden Cool. :)