[whatwg] Feedback on UndoManager spec

[Ojan Vafai]

On Thu, Oct 27, 2011 at 3:10 PM, Jonas Sicking <jonas at sicking.cc> wrote:

> On Thu, Oct 27, 2011 at 11:28 AM, Ojan Vafai <ojan at chromium.org> wrote:
> > On Thu, Oct 27, 2011 at 10:48 AM, Aryeh Gregor <ayg at aryeh.name> wrote:
> >>
> >> On Wed, Oct 26, 2011 at 2:39 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:
> >>
> >> > The same is true for having apply and reapply. Jonas wanted to get rid
> >> > of
> >> > reapply altogether and just have
> >> > void apply(in boolean isReapply)
> >> > In this world, you could do
> >> > { apply: function(isReapply) { return isReapply ? this.doApply() :
> >> > this.doReapply(); } }.
> >> > I didn't want this API because I'd expect apply and reapply to be
> >> > substantially different.
> >>
> >> I think either one of those two APIs would be fine.  I don't think the
> >> compromise is good, because it gives authors two ways to do the same
> >> thing, which is confusing.  I don't know which API is better without
> >> knowing the use-cases for manual transactions.  But Jonas' version is
> >> more flexible, because if the two are substantially different you can
> >> always just do
> >>
> >>  { apply: function(isReapply) {
> >>      if (isReapply) {
> >>          // reapply logic
> >>      } else {
> >>          // totally separate apply logic
> >>      }
> >>  }, . . . }
> >>
> >> which is really no harder to write than
> >>
> >>  { apply: function() {
> >>      // apply logic
> >>  }, reapply: function() {
> >>      // totally separate reapply logic
> >>  }, . . . }
> >>
> >> It's only one or two lines longer, and one level of indent greater.
> >> So I don't think a separate reapply member of the dictionary is
> >> useful.  It just makes things more complicated, even if most of the
> >> time the logic will be totally separate.
> >
> > I disagree. I think the boolean makes things more complicated. Boolean
> > arguments stink. Every time you want to use this API you need to go look
> up
> > the documentation to remember whether the boolean is "isReapply" or
> > "isApply". There's no such confusion if you have a separate method.
>
> Why is it harder to remember one function name and one attribute name,
> than to remember to function names?
>

If I write the following code, it executes, but does the opposite of what I
think it's doing:
{ apply: function(isApply) { //some logic using isApply; } }

With apply/reapply functions, the reapply method wouldn't execute at all.
That seems easier to diagnose to me.

This isn't as bad as cases like addEventListener where the boolean passed in
isn't required to be named, but it still just seems worse to me to have a
boolean argument. Maybe this is just a personal preference. This is
certainly deep in bikeshed territory. I don't much care about this one
instance, but I wouldn't be a fan of littering the web platform with boolean
arguments.