ceylon / ceylon-spec

DEPRECATED
Apache License 2.0
108 stars 34 forks source link

Inherit super class initializer parameters #892

Open djeang opened 10 years ago

djeang commented 10 years ago

Repeating class initialiser parameters from super class violates somehow DRY principle. This comes especially obvious when implementing classes having bunch of initialiser parameters spread along a consequent class hierarchy, as in CeylonFX.

shared class CheckBox (

    // From  CeylonFxAdapter
    JCheckBox delegate = JCheckBox(),

    // From Node
    String|Unset id = unset,
    String|Unset style = unset,
    BlendMode|Unset blendMode = unset,
    CacheHint|Unset cacheHint = unset,
    Node<JNode>|Unset clip = unset,
    Cursor|Unset cursor = unset,
    DepthTest|Unset depthTest = unset,
    Effect|Unset effect = unset,
    Boolean|Unset focusTraversable = unset,
    Location|Unset location = unset,
    Boolean|Unset managed = unset,
    Boolean|Unset mouseTransparent = unset,
    Boolean|Unset pickOnBounds = unset,
    Float|Unset rotate = unset,
    Point3D|Unset rotationAxis = unset,
    [Float, Float, Float]|Unset scale = unset,
    [Float, Float, Float]|Unset translate = unset,
    Boolean|Unset visible = true,

    // From control 
    // TODO ContextMenu
    Float|Unset maxHeight = unset,
    Float|Unset maxWidth = unset,
    Float|Unset minHeight = unset,
    Float|Unset minWidth = unset,
    Float|Unset prefHeight = unset,
    Float|Unset prefWidth = unset,
    // TODO skin
    Tooltip|Unset tooltip = unset,

    // From Labeled
    Position|Unset alignment = unset,
    // TODO ContentDisplay
    String|Unset elipsisString = unset,
    Font|Unset font = unset,
    Node|Unset graphic = unset,
    Float|Unset graphicTextGap = unset,
    Boolean|Unset mnemonicParsing = unset,
    TextAlignment|Unset textAlignment = unset,
    Paint|Unset textFill = unset,
    // TODO textOverrun
    String|Unset text = unset,
    Boolean|Unset underline = unset,
    Boolean|Unset wrapText = unset,

    // From ButtonBase
    // TODO onaction

    // From CheckBox
    Boolean|Unset selected = unset,
    Boolean|Unset allowIndeterminate = unset,
    Boolean|Unset indeterminate = unset) 

        extends ButtonBase<JCheckBox>(delegate, id, style,blendMode, cacheHint, clip, cursor,
        depthTest, effect, focusTraversable, location, managed, mouseTransparent,
        pickOnBounds, rotate, rotationAxis, scale, translate, visible, maxHeight, maxWidth, minHeight, minWidth,
        prefHeight, prefWidth, tooltip, alignment, elipsisString, font, graphic, graphicTextGap, mnemonicParsing, 
        textAlignment, textFill, text, underline, wrapText) {

    shared Property<Boolean> selectedProperty = booleanWrappedProperty(delegate.selectedProperty(), selected);

    shared Property<Boolean> allowIndeterminateProperty = booleanWrappedProperty(delegate.allowIndeterminateProperty(), allowIndeterminate);

    shared Property<Boolean> indeterminateProperty = booleanWrappedProperty(delegate.indeterminateProperty(), indeterminate);   
    }

This DRY violation occurres across the whole class hierarchy (ButtonBased, Labeled, Control, Node) and lead to a quite messy implementation/maintenance for such classes.

I would expect to leverage from a declaration as :

shared class CheckBox (

    // Import implicitly parameters declared in super class as 'extends' declares super class 
    // initialiser using {} intead of ()
    JCheckBox delegate = JCheckBox(), // Possible to refine parameters from super class

    // Need to declare only parameters specific for this class 
    Boolean|Unset selected = unset,
    Boolean|Unset allowIndeterminate = unset,
    Boolean|Unset indeterminate = unset) 

        // managed parameter (from Node) has not been implictly imported as it is explictly refined in super class initialiser 
        extends ButtonBase<JCheckBox>{managed = false;} {

    shared Property<Boolean> selectedProperty = booleanWrappedProperty(delegate.selectedProperty(), selected);

    shared Property<Boolean> allowIndeterminateProperty = booleanWrappedProperty(delegate.allowIndeterminateProperty(), allowIndeterminate);

    shared Property<Boolean> indeterminateProperty = booleanWrappedProperty(delegate.indeterminateProperty(), indeterminate);   
}

Surprisingly, I didn't find any related issue for such feature. Did i miss something ?

lucaswerkmeister commented 10 years ago

For named arguments in extends, see discussion here; in short: unsupported because then X extends Y { ... } gives error "missing class body" and (probably) "invalid named arguments invocation" but you really meant extends Y().

lucaswerkmeister commented 10 years ago

As for the issue of inheriting initializer params, I would suggest:

class A(String s="s") {}
class B(s) extends A(s) {}

that is, in the parameter list, B.s looks like a forward-specified parameter, but as it’s missing in the class’s initializer section, it’s inherited instead from A, including the default value.

@gavinking seems to have suggested something like that here, unfortunately with the comment “that this is not going to be something we can support” :(

djeang commented 10 years ago

Ok, Thanks for the links.

From these inputs, I see nothing fundamental preventing to do so, as soon as we accept some syntax accommodation. Maybe :

class A(String a, String b, String c) {...}
class B(super, String c="c", String d) extends A(;a ="forcedValue")  {...}   // ˋ(;ˋ means we use named args and reuse args declared in B constructor.
// shortcut for  class B(String b, String c="c", String d) extends A ("forcedValue", b,c) {...}

class B(super) extends A(;) {...}
// shortcut for class B(String a, String b, String c) extends class A(a, b, c)  {...}

Which may be not ideal but still better than forcing user to repeat super class parameters.

I also get that it was not trivial to implement but i am sure Ceylon team had already solved much challenging issues.

renatoathaydes commented 10 years ago

Just want to throw my support at this proposal.

This issue has made it really painful to implement CeylonFX properly.

In my opinion, there's nothing wrong with allowing a sub-class to inherit the default value of a parameter with same type and name. It is all about extension, inheritance, re-usability.

The special syntax above might not even be necessary.

class A(String a, String b="b", String c="c") {...}
class B(String a, String b,       String c="force", String d="d") extends A(a, b, c)  {...} 
value a = B("hi"); // a.a == "hi", a.b == "b", a.c == "force"

Pretty obvious what's going on.

It may make the compiler more complex, but that's about it for cons as far as I can see.

The argument that a class could break if it sub-classes a class from a different module when it is changed does not stand because that could happen in so many other ways just the way it is.

renatoathaydes commented 10 years ago

@gavinking said:

Look, constructor chaining is most akin to one function calling another function.

class A extends B === class A refines B

Class constructors are not functions.

If class B has a property x, then A inherits it unconditionally. It might "refine" it, but only if it does so with the same type or a sub-type.

If you have a constructor A(x), it would be outrageous to suppose you're not actually referring to the property x. You might do some checks or whatever before assigning the parameter value to property x, but still, they refer to the same thing.

Now, is it not natural to suppose that calling A(x) and B(x) both have the result of setting property x?

A constructor that maliciously contains a parameter with same name and type as a super-class, but doesn't actually refer to the same "thing", is clearly a terrible idea and if the language could stop that, it probably should!

Given the above, I say that your argument against this feature is not a valid one.

gavinking commented 10 years ago

class A extends B === class A refines B

I'm using "refines" in a particular technical sense. Another word for this is "overrides", though I don't especially like that word. Refinement/overriding is not subtyping.

Class constructors are not functions.

They absolutely, 100% are. Certainly they are in Ceylon. They are even assigned a function type by the language spec.

If class B has a property x, then A inherits it unconditionally.

Right, but a constructor is not a member of a type. It is a function that produces an instance of a type. If a supertype has a constructor with a certain signature, its subtypes are not required to have constructors with the same signature.

lucaswerkmeister commented 10 years ago

[disclaimer: I wrote this before @gavinking’s comment up there.]

@gavinking said:

There is no relationship between the parameters of one constructor and the parameters of the supertype constructor it calls.

There is not necessarily a relationship, but in many cases, CeylonFX being one, there is such a relationship.

The subtype constructor can have a totally different list of parameters with parameters in a different order, with different names, different semantics, etc.

Again, it can have a different list, but I agree with @renatoathaydes that in many (although not all) settings, this would be very unexpected to the user of the class, and therefore shouldn’t be done.

@renatoathaydes said:

Class constructors are not functions.

Actually, in a way they are. The following is well-typed:

void run() {
    variable String({Character*}) id = `String`; // refers to the constructor
    String a = id({'a'}); // invokes the constructor
    id = ({Character*} cs) => id(cs); // refers to an inline function
    String b = id(a); // invokes the function
}
gavinking commented 10 years ago

@lucaswerkmeister you don't even have to use the quotes.

String({Character*}) createString = String;

Or even:

 String createString({Character*} chars);
 createString = String;

Constructors are definitely functions in Ceylon, and the whole language is designed with that mental model.

lucaswerkmeister commented 10 years ago

you don't even have to use the quotes.

Neat, I didn’t know that. (Although it definitely makes sense, as you don’t have to use them for normal functions either.) Thanks!

gavinking commented 10 years ago

There is not necessarily a relationship, but in many cases, CeylonFX being one, there is such a relationship.

What I mean is that there is no well-defined relationship within the language that the typechecker can safely reason about. Sure, some frameworks might follow certain patterns but those patterns are not enforced by the language and therefore any kind of default argument inheritance would be a totally ad hoc thing, and fragile.

lucaswerkmeister commented 10 years ago

Well, then it’s a problem of finding a syntax where the typechecker doesn’t have to guess anything.

In that case, @djeang’s proposal (super inherites an arbitrary amount of parameters) won’t work. If you look at my proposal however, I hope you’ll find that there’s nothing the typechecker has to guess :) The class still defines the name and order of its parameters, it just omits their type and default value (making them look like a forward-declared attribute), and the typechecker then grabs those from the superclass. This still allows you to make a parameter non-optional (specify the type without a default value) or remove it completely.

gavinking commented 10 years ago

@lucaswerkmeister

As for the issue of inheriting initializer params, I would suggest:

class A(String s="s") {}
class B(s) extends A(s) {}

Yes, your proposal could in principle work, because it's really saying that the parameter of the subclass is exactly the same declaration as the member of the superclass. I don't immediately see anything very problematic about that idea.

renatoathaydes commented 10 years ago

Constructors are definitely functions in Ceylon

You're talking about implementation, I was talking conceptually. If you think that conceptually, a constructor is a function, then please reconsider your position regarding factory methods in the other discussion :)

@lucaswerkmeister Your proposal seems very neat and seems to solve this problem. What I had in mind was actually something like that, but turns out I am bad at putting ideas into words. Thanks.

djeang commented 10 years ago

Well, i am happy that this issue get finally some attention.

I just wanted to point out that Ceylon was great to instantiate classes having a big bunch of properties (thanks to its named arguments feature) as far as those classes are not participating in class hierarchy cause extends keyword does support ordered argument only.

Even if @lucaswerkmeister proposal is a step ahead, we will still ending with declarations as

... extends ButtonBase<JCheckBox>(delegate, id, style,blendMode, cacheHint, clip, cursor,
        depthTest, effect, focusTraversable, location, managed, mouseTransparent,
        pickOnBounds, rotate, rotationAxis, scale, translate, visible, maxHeight, maxWidth, minHeight, minWidth,
        prefHeight, prefWidth, tooltip, alignment, elipsisString, font, graphic, graphicTextGap, mnemonicParsing, 
        textAlignment, textFill, text, underline, wrapText) { ...

What a pity that {} stands for both named arguments list and block enclosing. Can't the compiler be smart enough to figure out that it needs either '(arg list) {code}' or '{arg list} {code}' after extends MySuperClass declaration ?

lucaswerkmeister commented 10 years ago

Can't the compiler be smart enough to figure out that it needs either '(arg list) {code}' or '{arg list} {code}' after extends MySuperClass declaration ?

See the link I posted above. The compiler can be smart enough, but only at the cost of confusing error messages in some other cases (when you forget the argument list, which people coming from other languages are certain to do a few times), and others have said that the code can look kinda ugly (class X(...) extends Y { ... } { ... }). You can try to convince @gavinking that the benefit of named arguments is higher :)

Another point that just occured to me: @gavinking proposed a syntax for constructors in #796. He didn’t show how it works together with inheritance, but I suppose it might be something like this:

class Child /* no parameter list – no default constructor */
        extends Person /* no parameter list because no default constructor */ {
    shared new Child(String firstName, [Person, Person] parents)
        extends Person("``firstName`` ``parents[0].lastName``") {
        ...
    }
    shared new Clone(Child child) extends Person("")
        => throw Exception("You monster.");
}

Note how this looks like a named argument invocation – I fear this might lead to an ambiguous grammar :(

we will still ending with declarations as [...]

I see your point, but in my opinion that verbosity is necessary because subclasses should still have the possibility to remove constructor parameters, and because additional parameters to the superclass shouldn’t automatically propagate to subclasses. I also think that it’s a good thing that I can look at the code of a class and immediately see all of its constructor parameters without having to look at its superclass – remember:

We spend more time reading other's code than writing our own. Therefore, Ceylon prioritizes readability […].

djeang commented 10 years ago

I see your point, but in my opinion that verbosity is necessary because subclasses should still have the possibility to remove constructor parameters, and because additional parameters to the superclass shouldn’t automatically propagate to subclasses. I also think that it’s a good thing that I can look at the code of a class and immediately see all of its constructor parameters without having to look at its superclass – remember: ...

Indeed. I always though that kind of inheritance as a syntax sugar, not as an in-deep language construct. So using the super keyword inside an argument list would just mean "Copy in place all the parameters declared in super class/constructor not already declared here.". If we need to not propagate all super class/constructor arguments then just not use this sugar. May it be applicable to function refining as well.

shared Person {
    shared new Person(String firstName, String lastName, Date birth) {...}  
}

shared Child extends Person {
    shared new Child(super, [Person, Person] parents, String lastName = null)  
        extends Person{super; lastName = parents[0].lastName;}  {...}  
              // lastName is declared here so `super` substitute only `firstname= firstanme; birth=birth;`
}