Options hash for DisplayObject constructors

[pcantrell]

There's a feature I'd like, and am willing to implement. I'm wondering whether you would accept it.

Easel's API has you create a scene graph object, then set its properties:

var house = new createjs.Bitmap();
house.image = houseImage;
house.x = 800;
house.y = 768 - groundHeight - houseImage.height;
background.addChild(house);

Easel doesn't use the common Javascript idiom of accepting a params hash in the constructor. As scenes get complicated, that idiom would be really nice to have. More declarative & nestable code like this ends up being much more readable as scene size grows:

background.addChild(
    new createjs.Bitmap({
        image: houseImage,
        x: 800,
        y: 768 - groundHeight - houseImage.height
    })
);

No smattering of little vars, everything concise and nested readably.

Shape poses a special problem:

var circle = new createjs.Shape();
circle.x = 50;
circle.y = 100;
circle.graphics
    .setStrokeStyle(5, 'round')
    .beginStroke('black')
    .beginFill('red')
    .drawCircle(0, 0, r);
background.addChild(circle);

Properties like stroke width and color are mixed into the graphics object, which leads to little codeballs that can be nasty to decipher (and what •was• the order of those setStroke params again?!?). I propose a few special options for common stroke & fill options, plus a callback for the graphics:

background.addChild(
  new createjs.Shape({
      x: 50,
      y: 100,
      strokeWidth: 5,
      strokeCap: 'round',
      strokeColor: 'black',
      fillColor: 'red',
      graphics: function(g) { g.drawCircle(0, 0, r); }
  })
);

I've prototyped all of this in my project, and it cleans things up a lot!

Thoughts? If I can get together a tidy implementation that doesn't create a mess in all the Easel constructors, would you accept the pull request?

[davidkaufman]

+1 to these feature requests!

[wookiehangover]

👍

[samreid]

+1 I'm on pcantrell's team, and I would like to see this implemented in Easel too--I think it makes the code seem more declarative (rather than imperative) and easier to create nested display objects. KineticJS has a feature like this and it would be nice to see it in Easel too:

http://www.html5canvastutorials.com/kineticjs/html5-canvas-kineticjs-rect-tutorial/

      var rect = new Kinetic.Rect({
        x: 239,
        y: 75,
        width: 100,
        height: 50,
        fill: 'green',
        stroke: 'black',
        strokeWidth: 4
      });

      // add the shape to the layer
      layer.add(rect);

[pcantrell]

OK, here's a pull request. I've tried to implement this as cleanly as possible, but even so, I'm grafting a new args style onto existing constructors, so some bits are sticky.

Notes on the implementation:

• The new opts API behaves exactly as described in my first post above.

• The DisplayObject constructor now accepts an options arg. If given { foo: 'bar' }, it will look for either a property named "foo" or "setFoo" and do the appropriate thing. Subclasses pass the options on up. This means that we don't have zillions of lines like this.foo = opts.foo littered all over the code. If an option is just a normal property, DisplayObject handles it for free. Some subclasses, e.g. Shape, add special handling for certain opts that don't correspond to object properties.

• I've attempted to maintain full compatibility for classes like Text and Bitmap whose constructors already accepted args. Distinguishing old-style calls from a new-style options hash is the ugly bit. I didn't try to add support for combining old-style params and new-styles opts (i.e. you can't do "new createjs.Text('hello', 'Arial', { size: 15 })"). You have to pick one or the other. If initialize() receives a single argument, it inspects the type to determine whether it's an opts object. The new DisplayObject._checkForOptsArg() method handles this, and it's my least favorite part of my code.

• Things get a little confusing in spots because Easel does subclassing in such a way that the super-constructor gets called when the subclass is created. If you're interested, I can send another pull request to fix this…. For example, I first did this for Container.initialize:

var opts = this._checkForOptsArg(arguments, [createjs.SpriteSheet]);
this.DisplayObject_initialize(opts);  // Let the super-ctor set this.children = opts.children
this.children = this.children || [];  // Default to [] if there was no opts.children

That breaks, however, because if you subclass Container, MySubclass.prototype.children gets set to [], and now every instanceof of MySubclass shares the same list of children. Oops.

Here's the fix:

var opts = this._checkForOptsArg(arguments, [createjs.SpriteSheet]);
this.children = [];                   // First set default (and override any MySubclass.children that got littered about)
this.DisplayObject_initialize(opts);  // Allow super-ctor to override with opts.children, if present

Phew. Let me know if there are things you'd like me to improve here.

[duncanbeevers]

This seems kind of convenient, but I think this is the wrong kind of functionality to shovel directly into EaselJS.

Instead, I think this makes sense in a wrapper library that you use to augment your EaselJS entities. Keep the main codebase small and specific-purpose, leave the syntactic niceties to external libraries.

[pcantrell]

@duncanbeevers Should EaselJS not strive to have a usable, elegant API?

[duncanbeevers]

I think it's a noble goal. I just don't think going the all-the-options-in-a-big-hash is the right direction. Chainable APIs are expressive, and in some languages, are type-safe. Options-soup interfaces are notoriously difficult to maintain, especially when married to other constructor APIs, and offer (next to) no information to the developer or compiler. Elements of the options tend to accumulate varied meanings, which we're already seeing. Options like strokeWidth, stroke and strokeCap needing to be merged to generate a meaningful method call, or the graphics option being a function that needs to be invoked with a specific signature.

IMO, the convenience comes down to personal taste, and personal taste should live in personal libraries.

[samreid]

@duncanbeevers Can you clarify what you mean by chainable APIs and how it pertains to Easel? I presumed you meant something like:

var player = new Bitmap(myImage).setX(100).setY(200);

In Easel, I see this pattern in Graphics, but not in DisplayObject.

[duncanbeevers]

@samreid That's exactly what I mean. This type of interface is also called a Fluent Interface. In strongly-typed languages the types of the individual segment's parameters and return values can be observed by the compiler / linter.

[pcantrell]

This is Javascript.

[gskinner]

I kind of wanted to see how this discussion would play out. I don't believe that an options param is the best way to address this, for many of the reasons already stated.

What I would perhaps suggest as a compromise (I'm not certain I will roll this into the main library), is a method on DisplayObject that offers this type of functionality through a chained interface. Something like:

stage.addChild(new Shape()).setProps({x:30, y:50, alpha:0.2, etc:etc});

This provides the same shorthand, but doesn't require overloading constructors, fits within current models, and is a bit more flexible (ex. you can call it later if you want). Thoughts?

[pcantrell]

The setProps approach is nice. The implementation comes out a lot cleaner — no fussing with args to distinguish different constructor forms.

There are a few potentially confusing spots where arguments receive special treatment in the constructor that they don't receive if they're set later (e.g. DOMElement's htmlElement), but those confusions already exist if people try to set properties directly even without this patch.

I'll submit a separate pull request for this shortly. It's a much smaller patch, which hopefully makes it more appealing for the core lib.

As for "how the discussion plays out," I just want to point out that we have four upvotes vs. duncanbeevers. Many of his objections — everything about "information to the compiler" and "information to the developer" — centers around static typing, and is thus is completely irrelevant when we're talking about Javascript. If this were C++, then sure, we'd think about static optimization — but it isn't and so we don't. If this were Java, we'd think about making Eclipse provide nice type information — but it isn't and so we don't.

That's what I meant by "this is Javascript." This style of programming is a standard JS pattern. It's widely used, and works •just fine•.

[pcantrell]

Pull request for setProps() approach: #227

[samreid]

Pull request #227 looks good to me. But before making it public I would want to discuss the name for this method. I think I would prefer "set" or "setProperties" to "setProps".

[gskinner]

Actually, ".set" would work. I was thinking that it was a JS reserved word, but it doesn't appear to be.

[pcantrell]

I like ".set" too. I would have presumed it's ruled out, but no, its special meaning appears to be entirely contextual; even the following nonsense works in both FF and Safari:

o = { set set(val) { console.log(val * val); } };
o.set = 3;  // outputs 9

What a quirky language!

[davidkaufman]

Hi guys,

+1 for a chained .set() method

Thanks!

-dave

On Tue, Dec 4, 2012 at 5:13 PM, pcantrell notifications@github.com wrote:

I like ".set" too. I would have presumed it's ruled out, but no, its
special meaning appears to be entirely contextual; even the following
nonsense works in both FF and Safari:

o = { set set(val) { console.log(val * val); } };
o.set = 3; // outputs 9

What a quirky language!

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/219#issuecomment-11018148.

David Kaufman
The Erlbaum Group
817 Broadway, 4th floor
New York, NY 10003
646-775-4151 (office)
570-688-3650 (mobile)
646-688-4029 (fax)
dkaufman@erlbaum.net

[pcantrell]

Looks like .set is the consensus. I'll close this pull request. Those who want this feature, please upvote #227!