Force named arguments in constructors.

Force named arguments in constructors.

R=blois@google.com,efortuna@google.com,podivilov@chromium.org,sra@google.com,vsm@google.com

Committed: https://code.google.com/p/dart/source/detail?r=13833

[Anton Muhin, 2012-10-17 14:08:38]

https://codereview.chromium.org/11196020/diff/1/tests/html/query_test.dart
File tests/html/query_test.dart (right):

https://codereview.chromium.org/11196020/diff/1/tests/html/query_test.dart#ne...
tests/html/query_test.dart:14: final canvas = new CanvasElement(width: 200,
height: 200);
now one is forced to use named arguments.

I personally find it more readable, YMMV of course.

In this case I'd resort to another approach outlined by Stephen (I believe):

new CanvasElement()..width = 200..height=200

Thoughts?

[vsm, 2012-10-17 15:17:18]

My inclination here is to get rid of these optional arguments altogether and
rely on cascades as you show.

What's an optional and what's not in constructors feels arbitrary right now
(i.e., why not id or class).  Cascades would be more uniform and should also get
rid of some dispatch code (though I suppose this could be optimized away).

Do we have any cases where a readonly attribute can only be set in the
constructor?  I.e., a real final?

More generally, what should be the philosophy on converting positional optionals
to named ones?  Mixing the two is potentially confusing.  Using named params
also requires a stable name - which we might not get from IDL.

On 2012/10/17 14:08:38, Anton Muhin wrote:
> https://codereview.chromium.org/11196020/diff/1/tests/html/query_test.dart
> File tests/html/query_test.dart (right):
> 
>
https://codereview.chromium.org/11196020/diff/1/tests/html/query_test.dart#ne...
> tests/html/query_test.dart:14: final canvas = new CanvasElement(width: 200,
> height: 200);
> now one is forced to use named arguments.
> 
> I personally find it more readable, YMMV of course.
> 
> In this case I'd resort to another approach outlined by Stephen (I believe):
> 
> new CanvasElement()..width = 200..height=200
> 
> Thoughts?

[blois, 2012-10-17 16:23:30]

It seems to me that the named arguments are bit cleaner than the cascades, but
this is a good general question about named constructor arguments vs cascades. 

I spoke to Bob briefly about it and his feeling was that the named params are a
bit easier to use and that for very common properties it makes sense to have
them as named params.

Note that the recommendation is that cascades always be placed on new lines, so
it's actually:
new CanvasElement(width: 200, height: 200);

vs:
new CanvasElement()
  ..width = 200
  ..height = 200

My feeling is that properties which must be set under the vast majority of cases
should be constructor arguments. This is a bit more concise and offers some
guidance to the developer as to how to use the class.

On 2012/10/17 15:17:18, vsm wrote:
> My inclination here is to get rid of these optional arguments altogether and
> rely on cascades as you show.
> 
> What's an optional and what's not in constructors feels arbitrary right now
> (i.e., why not id or class).  Cascades would be more uniform and should also
get
> rid of some dispatch code (though I suppose this could be optimized away).
> 
> Do we have any cases where a readonly attribute can only be set in the
> constructor?  I.e., a real final?
> 
> More generally, what should be the philosophy on converting positional
optionals
> to named ones?  Mixing the two is potentially confusing.  Using named params
> also requires a stable name - which we might not get from IDL.
> 
> On 2012/10/17 14:08:38, Anton Muhin wrote:
> > https://codereview.chromium.org/11196020/diff/1/tests/html/query_test.dart
> > File tests/html/query_test.dart (right):
> > 
> >
>
https://codereview.chromium.org/11196020/diff/1/tests/html/query_test.dart#ne...
> > tests/html/query_test.dart:14: final canvas = new CanvasElement(width: 200,
> > height: 200);
> > now one is forced to use named arguments.
> > 
> > I personally find it more readable, YMMV of course.
> > 
> > In this case I'd resort to another approach outlined by Stephen (I believe):
> > 
> > new CanvasElement()..width = 200..height=200
> > 
> > Thoughts?

[Emily Fortuna, 2012-10-17 17:37:05]

I agree with Pete here.