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

Constructor this.arg syntax #97

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

RblSb
Copy link
Member

@RblSb RblSb commented May 27, 2022

class People
  final name:String;
  final age:Int;
  function new(this.name, this.age) {}
}

Rendered version

@nadako
Copy link
Member

nadako commented May 29, 2022

I like it and I think we actually discussed this idea on the dev Slack before, but I struggle to remember if we had any unclarities or something against it...

@nadako
Copy link
Member

nadako commented May 29, 2022

I just found a discussion that this actually comes from Dart language, and also a link to a macro I wrote to implement similar thing: https://gist.github.com/nadako/273497023683e20f964ccd6ac0643422

@Aurel300
Copy link
Member

How do we express this in the AST? An extra (nullable) flag on haxe.macro.FunctionArg that can only be used on constructors?

@skial skial mentioned this pull request Jun 1, 2022
1 task
@back2dos
Copy link
Member

back2dos commented Jun 9, 2022

There's still a bit of redundancy, that I would quite like getting rid of.

Consider a somewhat complex class, with members grouped by concerns:

class Airplane {
  var captain:Pilot;
  var copilot:Pilot;
  // ... and more crew related stuff
  var thrust = .0;
  var pitch = .0;
  var roll = .0;
  var yaw = .0;
  var lat:Float;
  var lang:Float;
  // ... a dozen of functions to control the aircraft's rotation/position/acceleration
  // ... radio
  // ... baggage
  // ... and so on ...
}

But there is only one constructor. So one has to go back and forth between that and declarations to see which ones are initialized in it.

In tink_lang you can for example write var lat:Float = _;. I guess the concrete syntax is a matter of taste, but it avoids spreading things out. It could be new var lat:Float; or something.

@RblSb
Copy link
Member Author

RblSb commented Jun 11, 2022

I like new final foo:Float, this is less flexible, so you cannot change order of fields independent of argument order, but at the other side arguments order is easier to follow. But i think this needs something in constructor, to get attention for existed new fields, like:
function new(this.args, bar:String) {}
So you also can add non-field args before or after.

This also opens question, if we should assign null from argument to field with default value:
new final count = 0;

@back2dos
Copy link
Member

Hmmm. Valid points.

In the spirit of assigning arbitrary meanings to _ tink_lang allows also using it to specify where auto generated constructor args go, so in function new(foo:String, _, bar:String) they would go in the middle (there's no finer grained control).

This also opens question, if we should assign null from argument to field with default value:
new final count = 0;

Yeah, that's a good point. That should probably make 0 the default value (otherwise it's meaningless).

In the end short hands and complex classes as the example I gave before are perhaps not the best fit, so maybe I'm pushing this in a somewhat useless direction ;)

@nanjizal
Copy link

nanjizal commented Jul 1, 2022

@:structInit is awkward to remember hard to google and verbose, and where I would like short cut most, I am still seeing repetition, it could be simpler.

class People_ {
    public function new( public final this.name: String, private var this.age: Int ){}
}
abstract People( People_ ){
   public inline function child(){
       return this.age < 16;
   }
}
function main(){
    var boy: People = { name: Oliver, age: 12 };
    if( boy.child() == true ) trace( '$(boy.name) is child' );
}

@nanjizal
Copy link

nanjizal commented Jul 1, 2022

oops forgot the 'new'.. but may have over simplified from real haxe anyway.
You probably don't need 'this'... with my thought.. This should allow typedef construction by default as option.

class People_ {
    public function new( final name: String, public var age: Int ){}
}

@nanjizal
Copy link

nanjizal commented Jul 1, 2022

for lots of properties splitting by line would work fine.

class People_ {
    public function new( 
      final name: String
    , public var age: Int ){}
}

@TheDrawingCoder-Gamer
Copy link

Kotlin allows the paramaters to be put in the class header

 EnergyStoragePlugin(private val energyStorage: EnergyStorage)

@rhysuki
Copy link

rhysuki commented Jul 14, 2022

@nanjizal I'm not a fan of this. it trades clarity for brevity, adding a totally different spot for something as fundamental as field declaration to go into. it could introduce sneaky fields if you ever want to mixmatch between regular declaration and in-constructor declaration:

class Person {
  public var name: String;
  public var age: Int;
  public var friends: List<Person>;

  public function new(this.name, age: Int, ?friends: List<Person>, var id: Int) {
    this.age = age;
    this.friends = friends ?? new List<Person>();
  }
}

@player-03
Copy link

But there is only one constructor. So one has to go back and forth between that and declarations to see which ones are initialized in it.

The thing is, that's true with or without the new syntax.

class Airplane {
  var captain:Pilot;
  var copilot:Pilot;
  // ... and so on ...
  public function new(captain:Pilot) {
    this.captain = captain;
  }
}

captain is initialized, copilot isn't, and you have to scroll down to find that out. But there's also already a couple solutions available:

class Airplane {
  final captain:Pilot;
  var copilot:Null<Pilot>;
  // ... and so on ...
}

Now you know that captain must be set, and you should expect copilot to be null sometimes.

Alternatively, just write comments. I'd argue that if you're writing a class that's big enough for this to be a problem, you can take responsibility for code clarity. Haxe doesn't need to force it, at least not at the expense of constructor usability. (More on that later.)

I'm not a fan of this. it trades clarity for brevity, adding a totally different spot for something as fundamental as field declaration to go into.

Agreed. If I was searching for a certain field, I would definitely not think to check the constructor.

I do want shorter code, but not at the expense of clarity.


So at this point I've spoken out both for and against clarity. Evidently this isn't a simple issue. Let's talk about that.

I see this as a tug-of-war between brevity and clarity/usability.

  • Brevity is the whole point of this proposal. Existing code is longer than it needs to be, and we'd like to fix that with some syntax sugar.
    • The original proposal saves one line per variable (this.varName = varName), while keeping the constructor arguments about the same (you have to add this. but can remove the type hint).
    • The tink_lang approach removes even more code. You no longer need to type out the constructor arguments (beyond a single _ for all of them). You do have to mark the field declarations in some way (possibly with = _ or new final).
    • The "declare vars in constructor args" approach produces the shortest code. You only need to type each variable once, without so much as an extra _.
  • Clarity and usability come in many forms, and depend on use case.
    • Shorter code is often clearer and easier to write. That means the "constructor args" approach has a built-in advantage.
    • As rhynomatt described above, if you mix regular declaration and constructor args declaration, suddenly the code becomes a lot less clear. Still easy to write, noticeably harder to read.
    • As back2dos described, if you only have regular declarations, then you need to scroll down to the constructor to see which ones are initialized. Whereas if you mark them with = _ or new final, then you can instantly see which is which.
    • If you take the tink_lang approach and include a placeholder in the constructor arguments, then the constructor becomes less clear and less usable. Some users straight-up won't know what function new(foo:String, _, bar:String) means, and those who do will still have to scroll back up to the variable declarations to find which arguments to pass. (Hope they don't miss any!)
    • Speaking of the constructor signature, function new(this.name, this.age) doesn't tell you the variable types. name is easy enough to guess, but for age you'll have to scroll back up to check if it's an Int or Float. (Assuming for the sake of example that this is a large class.)
    • Users may want to write separate documentation for each instance variable, but if the variable is declared as a constructor argument, you'll have really limited access to formatting. Argument documentation isn't allowed to use line breaks, which in turn means you can't do paragraphs, lists, tables, or code blocks.

Aside: code completion (and related tools) can improve clarity/usability. However, it isn't always available, so I'd argue we shouldn't rely too heavily on it.

  • Code completion can tell you which arguments the constructor takes, even if the constructor definition omits type hints (like the original proposal) or includes a placeholder argument (like the tink_lang approach).
  • vshaxe could add an inlay hint to each variable declaration, so that you can see which are assigned via constructor argument without scrolling down.

@player-03
Copy link

player-03 commented Oct 17, 2022

My opinion: I prefer RblSb's original proposal over the others, but I find it hard to read and I'm not ready to endorse it.

I feel like I'd have trouble justifying any of the proposals to a newbie Haxe programmer. They're all hard to decipher at first glance. Why are some of these arguments prefixed with this. and then never referenced? Why do some of the arguments say var and final, and are also never referenced? What are all these underscores?

Given that we already have @:structInit, I'm starting to think that's all we need. At the very least, it's easier to Google than any of these other options.

Edit: I take that back. If @:structInit generates a constructor, the constructor won't be inline. That's kind of useless then.

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