HaxeFoundation / haxe

Haxe - The Cross-Platform Toolkit
https://haxe.org
6.16k stars 657 forks source link

Private getters/setters #3053

Open kevinresol opened 10 years ago

kevinresol commented 10 years ago

Sometimes we need a getter or setter, but only privately.

Example:

// this is readonly when outside the class, 
// but will go through the setter function when accessed within the class
public var myInt(default, private_set):Int; 

private function set_myInt(v:Int):Int
{
    // do something
    return myInt = v;
}
Simn commented 10 years ago

We cannot change the visibility interpretation just like this because it would break a lot of code. This should be a rare use-case for which it is fair to require you to manually call an accessor method.

J-d-H commented 10 years ago

How can it break existing code if there is no private_set?

kevinresol commented 10 years ago

@Simn Sorry for not making my point clear. I didn't mean to change the current visibility, instead I suggested to add a new accessor.

nadako commented 10 years ago

privateSet could do the job, however I doubt it actually worth the effort to implement, as the use case is indeed should be rare, because in most cases it's actually better readability-wise to explicitly call setter function (I think)

kevinresol commented 10 years ago
public var myInt(get, never):Int
private var myInternalInt(default, set):Int

private function get_myInt():Int return myInternalInt;
private function set_myInternalInt(v:Int):Int
{
    //do something
    return myInternalInt = v;
}

Now I have to do this, when outside the class I access myInt and when inside the class I access myInternalInt

To me, I think it is somewhat inconsistent and redundant to access different variables at different scopes.

@nadako not sure about the difficulty to implement this, I thought it is just a combination of null (visibility) and get/set (points to accessor method)

ncannasse commented 10 years ago

I'm pretty sure this can be done with a macro :)

kevinresol commented 9 years ago

Just a reminder that haxe still lacks complete accessor combinations

public direct read private direct read public with getter private with getter
public direct write public (default, default) public (null, default) public (get, default) ?
private direct write public (default, null) private (null, null) public (get, null) private(get, null)
public with setter public (default, set) public (null, set) public (get, set) ?
private with setter ? private (null, set) ? private (get, set)
nadako commented 4 years ago

I'm stumbling upon this issue from time to time and it is pretty annoying that we don't have "private set".

This should be a rare use-case for which it is fair to require you to manually call an accessor method.

Normally I would agree, but this problem often arises when using macros to build all kinds of "change-tracking" data classes. For example, consider we have something like:

class Player implements Magic /* @:autoBuild here */ {
    public var health:Int;
    public function damage(amount) {
        health -= amount;
    }
}

what Magic transforms the class into is something like:

class Player {
    public var health(default,set):Int;
    public function damage(amount) {
        health -= amount;
    }
    function set_health(value) {
        this.health = value;
        dispatchChange(); // or markDirty or whatever
        return value;
    }
}

So far so good, but imagine user wants to forbid writing to health from outside. Naturally they would do var health(default,null):Int, which brings us to the second point:

I'm pretty sure this can be done with a macro :)

Just because there's no accessor-method counterpart to default,null, the macros has to become much much more complicated and slower. Now, to achieve the code above while keeping access control the same, a macro needs to:

Can we please reconsider in view of the problem I described above?

nadako commented 4 years ago

all kinds of "change-tracking" data classes

FWIW this must be an issue for hxbit as well. If not, I'm very curious how it is solved.

RealyUniqueName commented 4 years ago

I agree we need a solution for this. What about var field(private get, private set):Int ? But that would probably require to deprecate null and default accessors in favor of something like private and public to be consistent (e.g. var field(public,private) instead of var field(default,null))

back2dos commented 4 years ago

I'm very much in favor, although this seems like a bad time to start deprecating accessors. Since the solution is primarily intended for macros, perhaps a solution based on field metadata would be an adequate path to support this without breaking changes.

RealyUniqueName commented 4 years ago

I've faced this issue out of a macro context a few times. Being forced to introduce additional fields doesn't feel good. So I'd like to have a "complete" solution not only concerning macros.

kLabz commented 4 years ago

Could we get private get and private set (and probably private alone for null) without null/default deprecation as a first step?

nadako commented 4 years ago

While I think we should change null to private (because it's a private-only access after all), changing default to public doesn't make sense to me, because there can be private var x(public,set), but yeah, that's a separate discussion IMO.

Also, I'm not sure if it should be private set or privateSet. The former is not a valid identifier/expression, which might limit us in future with whatever we want to do with properties.