Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default implementations in the interfaces #54

Closed
wants to merge 8 commits into from
Closed

Default implementations in the interfaces #54

wants to merge 8 commits into from

Conversation

posxposy
Copy link

@posxposy posxposy commented Jan 19, 2019

interface IUser {
    //Fields marked as default should be initialized here;
    default private var email:String = "";
    
    //Methods marked as default should be implemented here;
    default public function isEmailValid ():Bool {
        /* ... */ 
        return false;
    }
    
    //This method is still required to be implemented by childs
    public function doSomething ():Void;
}

class Student implements IUser {
    //This method is required by IUser
    public function doSomething ():Void {
        isValidEmail (); // this method can be used here, implementation will be injected from the interface
    }
} 

class Teacher implements IUser {
    //This method is required by IUser
    public function doSomething ():Void {  }

    //Discard default impl, and use it's own
    public function isValidEmail ():Bool {
        return false;
    }
} 

Rendered version

@ncannasse
Copy link
Member

What you might want here instead is similar to Java abstract methods, that forces children classes to implement certain methods defined in their superclass.

@posxposy
Copy link
Author

@ncannasse

What you might want here instead is similar to Java abstract methods, that forces children classes to implement certain methods defined in their superclass.

Well, that's a half of problem :) To be fair, I picked up default implementations from Java too. Both things are useful and flexible. Maybe it is possible to support both features? Abstract methods in classes and default implementations in interfaces.
That may improve Haxe as a language and made it more rich as for me, and reduce unnecessary boilerplate code in some situations.

@skial skial mentioned this pull request Jan 23, 2019
1 task
@Aurel300
Copy link
Member

I just wanted to add that this is already possible using macros, e.g. with tink_lang partial implementations (though implementing this manually in macros is pretty easy also).

But I think proper traits (which is essentially what this is, given that your default allows both variable and method fields) might be interesting to have without having to go into macroland.

@Simn
Copy link
Member

Simn commented Apr 6, 2019

I don't want code in interfaces.

I would be open to abstract methods Java-style though, though I didn't really think the implications through yet.

@RealyUniqueName
Copy link
Member

I agree with @Simn. Interfaces should stay interfaces.
And the proposed feature is better to be designed and implemented as "classic" abstract classes, which I'd like to see in Haxe type system some day.

@posxposy
Copy link
Author

posxposy commented Apr 8, 2019

@Simn

I would be open to abstract methods Java-style though

Well, this idea actually comes from Java too:

// A simple program to Test Interface default 
// methods in java 
interface TestInterface 
{ 
    // abstract method 
    public void square(int a); 
  
    // default method 
    default void show() 
    { 
      System.out.println("Default Method Executed"); 
    } 
} 

Example from here: https://www.geeksforgeeks.org/default-methods-java/

But, would be nice to see "classic" abstract classes (as @RealyUniqueName says) in Haxe instead of "default" things in interfaces. Why not :)

@hughsando
Copy link
Member

I think static functions/variables would be convenient in enums and interfaces, otherwise you have to make pairs of classes, which can be annoying .

@jcward
Copy link

jcward commented Apr 15, 2019

Just chiming in to say, I like interface default implementations, they're very handy. I have built a macro implementation, and it's a little slow and a little hairy - so in that respect, it'd be nice if it was a built-in feature.

In my implementation, I've (ab)used metadata, which happens to keep the implementation separated (in the metadata) out of the pure interface definition:

package;

@:autoBuild(util.DefaultImplementations.apply_defaults({
  some_var:"hello world",
  get_is_bar:
  {
    return some_var == "bar";
  },
  set_is_bar:
  {
    // Note: variable name 'input_value' is defined in the interface below
    if (input_value) some_var = "bar";
    return input_value;
  }
}))
interface IFoo
{
  public var some_var:String;

  public var is_bar(get,set):Bool;
  public function get_is_bar():Bool;
  public function set_is_bar(input_value:Bool):Bool;
}

The most hairy part is that, while it's building fields for the given class, it doesn't know which interface triggered the macro. So it scans all the interfaces of the class (and parent classes) to find the one matching the fields provided in the metadata. It needs the interface, because while the metadata provides the Expr, the interface provides the Field definition.

I say abused metadata, because technically I don't think metadata supports generic expressions per the manual... but it does work in both Haxe 3 and 4. Although vshaxe isn't always happy about it -- some if-defs have been necessary.

One downside of separating the interface definition from the implementation is that the method parameter names, e.g. input_value, is in the interface, and must be used in the implementation.

ETA: Oh, and it's sometimes handy to @:final a default method to disallow classes from providing an implementation:

  some_method: @:final {
    ...
  }

@Gama11
Copy link
Member

Gama11 commented Apr 15, 2019

I don't think metadata supports generic expressions per the manual...

I think that's perfectly valid:

arguments to compile-time metadata can be any valid expression.

@Simn
Copy link
Member

Simn commented Apr 16, 2019

This has been rejected by nay votes of @ncannasse, @nadako, @RealyUniqueName and @Simn.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants