On 11-08-09 6:36 PM, Jonas Sicking wrote:
On Tue, Aug 9, 2011 at 3:11 PM, Ryosuke Niwa<[email protected]> wrote:
On Tue, Aug 9, 2011 at 2:55 PM, Jonas Sicking<[email protected]> wrote:
I don't think it's a matter of which use cases can or can't be solved with
either solution. It's pretty clear to me that all scenarios can be solved
with either API.
Right, they're isomorphic.
It's just a matter of which pattern is more common and so which one we
should make more convenient. If almost everyone puts the same code in apply
and reapply then we're just creating more work for people.
Here's how you'd implement the apply/reapply/unapply syntax using simply
apply/unapply
apply: function() { if (!this.applied) { action1(); this.applied = true; }
else { action2(); }
unapply: unapply
One thing I don't like about this approach is that it requires a flag. With
my proposal, all you need to do is to call a function. Also, we can make it
so that when you don't supply a value to reapply (i.e. reapply is null),
then undoManager automatically calls apply instead (I always intended this
behavior from the beginning but I've apparently left this details out).
Sure, your API is more convenient in certain situations. But it also
encourages code duplication (I'll note that in the examples you
originally provided in this thread you always ended up duplicating
code between apply/reapply), which easily leads to bugs.
I think this is a very important point, and this downside makes me think
that we shouldn't expose a reapply API. Since almost every case that I
can think of where having a reapply function would make more sense
requires authors to maintain other state information in their
transaction objects, I don't think that maintaining one more boolean
flag is going to make things noticeably harder for them.
Ehsan
