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 get / default set for properties #11434

Open
kLabz opened this issue Dec 18, 2023 · 12 comments
Open

default get / default set for properties #11434

kLabz opened this issue Dec 18, 2023 · 12 comments

Comments

@kLabz
Copy link
Contributor

kLabz commented Dec 18, 2023

While we might add HaxeFoundation/haxe-evolution#96 in 5.0 (?), current property syntax could be enhanced with a new modifier which would be equivalent to public var propertyWithField:T { get; set; }:

public var propertyWithField(default get, default set):T;

This would be equivalent to:

@:isVar
public var propertyWithField(get, set):T;
function get_propertyWithField():T return propertyWithField;
function set_propertyWithField(val:T):T return propertyWithField = val;

Which allows child classes to override getter/setter.


Also while we're at it, maybe init like in C# would be a nice addition now that we have final fields with similar behavior 🤔

@Simn
Copy link
Member

Simn commented Dec 18, 2023

This looks like it should be an IDE feature, not a language feature. This kind of stuff always looks great in simple examples, but then when you want to make a small modification (or add documentation to your accessors) you have to revert to the verbose version again.

@kLabz
Copy link
Contributor Author

kLabz commented Dec 18, 2023

If you want to change the getter, you remove default and add the getter? That doesn't sound bad to me.

Also IDE save keystroke but it doesn't really save us from the visual bloat of pointless getter/setter walls

@Simn
Copy link
Member

Simn commented Dec 18, 2023

GEmZ7MS7VL

->

I2JpdusNs9

Haxe lifehacks episode 1 o.o

@back2dos
Copy link
Member

back2dos commented Dec 18, 2023

This looks like it should be an IDE feature, not a language feature.

Readability is not an IDE feature, it's a language feature.

The moment you open the diff in some tool or web interface another, there's no IDE to help hide the noise of code that carries 0 information. There's no information in function get_propertyWithField():T return propertyWithField; that's not more clearly conveyed by putting the default keyword at the suggested location. I just have to read it to see that indeed I've learned absolutely nothing.

This kind of stuff always looks great in simple examples, but then when you want to make a small modification (or add documentation to your accessors) you have to revert to the verbose version again.

You do not need documentation for default accessors, because there's nothing to be documented. And when you do want to modify an accessor, then yes, that's where the IDE would come in, by indeed offering a refactoring action that expands default get into get + the getter.

So yes, IDEs can help in generating code when it makes sense. However, it just makes no sense to have trivial accessors spelt out, because it bring no value, only distraction.

We can probably agree that said cognitive load is trivial and that introducing this would therefore solve nothing that requires solving. I won't contest that. But if we were to agree that this should be solved, then the IDE is absolutely not the place to do it,

@Simn
Copy link
Member

Simn commented Dec 19, 2023

Readability is not an IDE feature, it's a language feature.

I agree, but I don't accept that this is more readable. There may be less to read, but without any intuition for what default get means, I argue that this is actually harder to read.

@player-03
Copy link
Contributor

player-03 commented Dec 19, 2023

What if you could leave out the body of the accessor?

@:isVar public var property(get, set):T;
function get_property():T;
function set_property(val:T):T;

This removes some of the repetitive code without messing with established keywords. Sure it doesn't remove as much code, but on the plus side, it's a lot more flexible:

@:isVar public var property(get, set):T;
inline function get_property():T;
@:noCompletion public function set_property(val:T):T;

@player-03
Copy link
Contributor

Actually, all of this could be a build macro. For each property field, check if the accessors exist. If not, add them.

@back2dos
Copy link
Member

Readability is not an IDE feature, it's a language feature.

I agree, but I don't accept that this is more readable. There may be less to read, but without any intuition for what default get means, I argue that this is actually harder to read.

Initially, you said this looks great, but should be done in the IDE, because it doesn't work for edge cases. Now you're claiming that the syntax is unintelligible. Could I please bother you to structure your reasoning just a little bit? :P

Also. the way you're phrasing it, it sounds like the kind of discovery that people make after the 3rd joint. "Does anything have any meaning?".

We simply have to assign common meaning to things, because otherwise we get nowhere. It's far from self evident what @:isVar public var property(get, set):T even means, like wtf is @:isVar and why can it be dropped for public var property(default, set):T?

I think the proposed solution is certainly no worse. When get means "has a getter" , then default get meaning "has a default getter" does not seem outrageously surprising or even ambiguous.

Of course it's a trade off, as it adds to the number of stuff a Haxe user has to know about. So we have to ask: will anyone actually be using this? I probably have no use for it. But if people say they have use cases for this, then I suppose we can trust them ^^

My primary worry is about how this will be represented in build macros without breaking anything. Perhaps it should already be expanded?

@Simn
Copy link
Member

Simn commented Dec 20, 2023

I do like how this looks, but I still don't find it more readable. Both of these statements can be true at the same time. I also never said it was unintelligible, I'm just refuting this readability angle you guys started.

@ALANVF
Copy link
Contributor

ALANVF commented Dec 23, 2023

In order to maybe make this a bit more useful, what if we extended the syntax like this?

var foo(get = default, set = default): Int;

It makes what's going on very clear and straightforward, and the syntax would allow you to provide a custom function instead of default, similar to what used to be allowed for accessor aliasing (or so I hear):

var foo(get, set): Int;
var otherFoo(get, set = set_foo): Int;

@AndrewDRX
Copy link

If there is a plan to implement the C#-esque syntax/functionality as documented in the linked proposal, then is there actually a need to also enhance the current syntax with a new modifier?

If the current syntax is to be deprecated as the proposal requests, then it seems confusing to continue enhancing a deprecated syntax.

Unless depreciation is not on the table, then it seems confusing to be supporting two different syntaxes.

@Geokureli
Copy link

Geokureli commented Mar 25, 2024

While were discussing C# style getter/setters, this would also allow for public getters and private setter functions, though we also could add this by allowing:

public var foo (get, private set):Int;

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

No branches or pull requests

7 participants