Implementation of Asymmetric Visibility for Properties in PhpGenerator

David Grudl
Nette Core | 8228
+
+2
-

I'm working on implementing asymmetric visibility for properties in Nette PhpGenerator (e.g., public getter + protected setter). I have 3 API design variants, and I’m interested in your opinion.

Variant 1: Boolean parameter

$prop->setPublic(); // default for getter
$prop->setProtected(set: true); // for setter only
if ($prop->isPrivate(set: true)) {
    // check the setter
}

✅ Intuitive boolean choice
✅ IDE suggests named parameter
✅ Consistent API pattern

❌ Lacks the ability to write (get: true)
❌ Potential confusion with $prop->setPublic(set: false)

Variant 2: Separate methods

$prop->setPublic(); // default for getter
$prop->setPublicSet(); // visibility for setter
if ($prop->isPrivateSet()) {
    // check the setter
}

✅ Explicit separate methods
✅ Similar to Reflection API

❌ Duplicate methods
❌ Awkward naming (getVisibilitySet?)
❌ Awkward naming II. – getVisibility() would need to be replaced with getVisibilityGet()

Variant 3: String mode parameter

$prop->setPublic(); // or setPublic('get')
$prop->setProtected('set');
if ($prop->isPrivate('set')) {
    // check the setter
}

✅ Readable in code
✅ Flexible for possible additional modes (though they’re not expected)

❌ Prone to typos
❌ PhpStorm doesn’t suggest (yet, but PHPStan can validate)
❌ String literals in code (a constant/enum could be created for this)


I’m leaning toward Variant 3, but I’d like to hear your opinion. Which would you choose and why?

Šaman
Member | 2666
+
0
-

I've never used it before, but the third example seems the most readable to me.

But maybe when it's in common use, a different best practise will be used.

Last edited by Šaman (2024-11-28 16:45)

Marek Bartoš
Nette Blogger | 1275
+
0
-

If you don't require PHP 8.1 just yet – here is my solution replicating enum behavior. Once I require 8.1+ I'll just turn the class into enum, deprecate constructor methods and replace them with enum cases. It would fit nice with the third variant.

Btw, will the new feature support get/set expressions?

David Grudl
Nette Core | 8228
+
0
-

@MarekBartoš wrong thread?

dkorpar
Member | 136
+
0
-

Enums actually sounds pretty ok for me to here…

enum VisibilityMode: string
{
    case GET = 'get';
    case SET = 'set';
}

$prop->setProtected(VisibilityMode::SET);

You get option three without all the cons…

Last edited by dkorpar (2024-11-28 21:00)

Marek Bartoš
Nette Blogger | 1275
+
0
-

@DavidGrudl No. I am proposing to use the third variant, but with enum-like class. It would be strictly typed and could be turned into real enum once the php-generator requires PHP 8.1+

It would be just used as $prop->setProtected(AccessMode::set()); and then, with PHP 8.1+ required $prop->setProtected(AccessMode::Set);


This is the class

use ValueError;

final class AccessMode
{

	private const Set = 1,
		Get = 2,
		SetGet = 3;

	private const ValuesAndNames = [
		self::Set => 'Set',
		self::Get => 'Get',
		self::SetGet => 'SetGet',
	];

	/** @readonly */
	public string $name;

	/** @readonly */
	public int $value;

	/** @var array<string, self> */
	private static array $instances = [];

	private function __construct(string $name, int $value)
	{
		$this->name = $name;
		$this->value = $value;
	}

	public static function set(): self
	{
		return self::from(self::Set);
	}

	public static function get(): self
	{
		return self::from(self::Get);
	}

	public static function setGet(): self
	{
		return self::from(self::SetGet);
	}

	public static function tryFrom(int $value): ?self
	{
		$key = self::ValuesAndNames[$value] ?? null;

		if ($key === null) {
			return null;
		}

		return self::$instances[$key] ??= new self($key, $value);
	}

	public static function from(int $value): self
	{
		$self = self::tryFrom($value);

		if ($self === null) {
			throw new ValueError();
		}

		return $self;
	}

	/**
	 * @return array<self>
	 */
	public static function cases(): array
	{
		$cases = [];
		foreach (self::ValuesAndNames as $value => $name) {
			$cases[] = self::from($value);
		}

		return $cases;
	}

}

And once you require PHP 8.1+

enum AccessMode: int
{

	case Set = 1;

	case Get = 2;

	case SetGet = 3;

	/**
	 * @deprecated
	 * @use self::Set
	 */
	public static function set(): self
	{
		return self::Set;
	}

	/**
	 * @deprecated
	 * @use self::Get
	 */
	public static function get(): self
	{
		return self::Get;
	}

	/**
	 * @deprecated
	 * @use self::SetGet
	 */
	public static function setGet(): self
	{
		return self::SetGet;
	}

}

Last edited by Marek Bartoš (2024-11-28 22:21)

David Grudl
Nette Core | 8228
+
0
-

The third case can, of course, be solved using a constant or an enum; I mention this very briefly there. I consider this a minor detail.

mystik
Member | 312
+
+1
-

I would prefder 2 or 3 with enums.

But maybe it would be more readable using names Setter/Getter and not only set/get. Like getSetterVisibility()

mystik
Member | 312
+
+1
-

And anothet point. I would expect setVisibility to set both setter and getter visibility by default. And speciakised setSetterVisibikity and setGetterVisibility (or with exokicit param) for individual settingd