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

Private getters/setters #3053

Open
kevinresol opened this issue May 27, 2014 · 14 comments
Open

Private getters/setters #3053

kevinresol opened this issue May 27, 2014 · 14 comments
Milestone

Comments

@kevinresol
Copy link
Contributor

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
Copy link
Member

Simn commented May 27, 2014

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.

@Simn Simn closed this as completed May 27, 2014
@J-d-H
Copy link
Contributor

J-d-H commented May 27, 2014

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

@kevinresol
Copy link
Contributor Author

@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
Copy link
Member

nadako commented May 27, 2014

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
Copy link
Contributor Author

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
Copy link
Member

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

@kevinresol
Copy link
Contributor Author

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
Copy link
Member

nadako commented May 27, 2020

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:

  • change property to get,never
  • add a backing field (which is an internal structure change that most probably affects e.g. serialization)
  • find ALL the assignments to the original field and change them to the corresponding set_ (which I think is impossible in general, because a) we need to type the method expressions and figure out that a field is referenced and b) we can"t affect child classes at this point, so we would have to store the info about such fields and run similar processing on each of the child class methods.

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

@nadako nadako reopened this May 27, 2020
@nadako
Copy link
Member

nadako commented May 27, 2020

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
Copy link
Member

RealyUniqueName commented Jun 1, 2020

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
Copy link
Member

back2dos commented Jun 1, 2020

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
Copy link
Member

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.

@RealyUniqueName RealyUniqueName added this to the Design milestone Jun 1, 2020
@kLabz
Copy link
Contributor

kLabz commented Jun 2, 2020

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

@nadako
Copy link
Member

nadako commented Jun 2, 2020

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.

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

8 participants