[RFC[ Property accessor hooks, take 2



Hi Larry,
It's good to see this idea still progressing.
I have to agree with the other comment(s) that the implicit
$field/$value variables seem odd to me. I understand the desire for
brevity and the potential future scope of reused hooks, but this
concept seems to fly in the face of many years of PHP reducing "magic"
like this.

The "magic" that PHP has been removing is mostly weird and illogical type casting. As noted, neither of these variables are any more "magic" than $this.

However, since it seems no one likes $field, we have removed it from the RFC. Of note, to respond to your comment further down, $this->{PROPERTY} will not work. The virtual-property detection looks for the AST representation of $this->propName, and only that. Dynamic versions that turn into that at runtime cannot work, as it needs to be known at compile time.

I guess it's mostly irrelevant on single-use hooks anyway, but that sounds like a potential gotcha with reusable hooks, and I think it's worth making it very clear now in the RFC/docs that dynamic access like this won't work as expected (and why). Perhaps some other indicator on reusablele hooks can be used at that point, to signify if it's virtual or not.

For $value, however, we feel strongly that having the default there is a necessary part of the ergonomic picture. In particular, given your comments here:

To give one answer to your question about ambiguity if the $value
parameter is required - I don't believe this is actually ambiguous, in
the context of PHP:

• method parameters in child classes don't implicitly 'inherit' the
  parent method parameter's type if they don't define one (they widen to
  mixed);
• method return types have no implicit inheritance, they must declare a
  compatible return type;
• typed class properties don't implicitly inherit the parent type when
  the type left off a child property - they must declare the same type.
  AFAIK there is no existing behaviour in PHP where omitting a type would
  mean "the type is implicitly inherited from X", it either means the
  same as mixed, or it's an error.

That to me suggests that IF a custom variable name is provided, we should require also specifying the type. In which case, in the 95% case, if we require the full argument signature then the 95% case would need to double-specify the type, which is a hard-no from an ergonomic standpoint.

Especially combined with the suggestion yesterday to allow return-to-set in the short-set version, that would mean comparing this:

public string $phone {
set(string $phone) => $this->phone = $this->sanitizePhone($phone);
}

To this:

public string $phone {
set => $this->sanitizePhone($value);
}

And to me, there's absolutely no contest. The latter has about 1/3 as many places for me to make a typo repeating the same information over again. Now imagine comparing the above in a property that's used with constructor promotion.

public function __construct(
public string $phone { set(string $phone) => $this->phone = $this->sanitizePhone($phone); }
public string $phone { set => $this->sanitizePhone($value); }
) {}

Again, it's absolutely no contest for me. I would detest writing the longer version every time.

I think you're making slightly misleading comparisons there, by picking the two extremes (and ignoring the possibly for shorter explicit names) - the explicit parameter name surely doesn't preclude the use of return-to-set functionality? So the comparison could equally be:

public function __construct(
   public string $phone { set => $this->sanitizePhone($value); }
   public string $phone { set(string $v) => $this->sanitizePhone($v); }
){}

If PHP has been moving away from weird and inexplicable magic, it's also been moving away from needless boilerplate. (Constructor promotion being the best example, but not the only; types themselves are a factor here, as are arrow functions.) As the whole point of this RFC is to make writing common code easier, requiring redundant boilerplate for it to work is actively counter-productive.

So what I'd suggest instead is "specify the full signature if you want a custom name OR wider type; or omit the param list entirely to get a same-type $value variable, which 99% of the time is all you need." We get that in return for documenting "$value is the default", which for someone who has already figured out $this, should be a very low effort to learn.

I get your point, and to expand on what I said in the first email - if removing the implicit mode would mean people vote against the RFC, then that's a worse result, IMO (this is why I suggest a secondary vote - perfect is th enemy of good and all that). I personally think the implicit variables will result in less-readable code, but I also know that I'm free to not use the implicit parameter in code that I write, so I trust those with a more accurate finger on the pulse of the voters to know whether the implicit $value parameter will help or hinder the RFC to pass.

Also, a small nitpick: The link to your attributeutils repo in the
examples page, is broken, and it would be nice to see a few examples
showing the explicit version of the hooks.

Link fixed, thanks. What do you mean explicit version of the hooks?

I meant hooks that don't use the implicit "magic" variables (i.e. specify the parameter on set() specify the full property name when accessing the backing store). Given your first response I guess this request is a little moot, assuming the examples are updated to remove use of $field. Maybe the RFC or an example already covers it and I skipped over it, but it'd be good to make it clear exactly what the equivalent of an implicit $value parameter is.

--Larry Garfield

Cheers

Stephen