Jump to content

Best practice for field references in PHP?


MarkE
 Share

Recommended Posts

To reduce maintenance effort, it seems to me to be important that there is a clear and manageable interface between names in the PW database and those in your PHP code. But what is the best way of achieving this? Firstly, I think the use of custom Page classes helps enormously, but then what? Two PW gurus to whom I usually look for inspiration seem to have different approaches. @Robin S appears to favour defining properties above the class, as is exemplified in his AutoTemplateStubs module. Alternatively, @bernhard uses constants to define fields (see here for example). Both work but have pros and cons. The first approach seems more straightforward and fits naturally into the normal PHP object->property syntax and PW selector syntax, but if you change a fieldname in the database, you have to get your IDE to refactor it in the code (PhpStorm seems to handle this fine - refactor the @property definition and all the accessed properties get changed). The second approach limits your syntax to an array-style, but means you just change the constant value if the field name changes - no refactoring required.

Any other views on this, or better ideas?

  • Like 1
Link to comment
Share on other sites

Don't overthink it. You're gonna need a way to reference a specific field anyway. Hiding the name away behind a constant, config or whatever isn't going to change that. If you're worried about changing field names, you should instead invest the time to find better names for your fields. Why would your field names change? Probably because the data they hold or the thing they represent have changed - both of those cases will require adjustments in the templates anyway.

I've never understood the benefit of "hiding" a property or method name behind a constant. I mean, what are you gonna do if the field name behind it changes? Let's say I have the field field_for_thing_a and use a constant FIELD_NAME_THING_A to hide the dirty, dirty field name string behind a nice, verbose constant. Now I need to rename the field to field_for_different_thing_b. Am I just gonna leave the constant in place? So FIELD_NAME_THING_A now references something completely different? No, I'm gonna rename it, in which case I could just find+replace the name of the thing itself and will introduce a BC break anyway. But if the field needs to change, it's not backwards compatible anyway. The only backwards compatibility I'm maintaining at this point is unreadable cargo cult code ?

Of course, there are exceptions. Depends on what you're building. If you're talking about a module which might need to access a user-defined field, by all means, store the field name in a variable (it's gonna be pulled from the database anyway if it's configured in the module settings). Maybe I'm way off base here. What problem are you actually trying to solve? Maintainability is a spectrum and doesn't exist in a vacuum. What maintenance burden do you want to avoid?

  • Like 2
Link to comment
Share on other sites

Your link to my example is wrong - I guess you mean this one ? 

HiHxiGI.png

The main advantage of using constants is not about being able to change names (as that happens almost never) but to get better structure into your code and also get additional information that you do not get from regular strings or object properties. And I'm using it a lot with array syntax but you are absolutely not limited to that! And you cal of yourse always use class constants in strings:

$pages->find("template=".Foo::tpl);

But here are some very simple examples why I'm using that approach in the first place. Imagine you wrote code 1 year ago and look at it to fix a bug or add a feature. Compare those two:

// example migration
$rockmigrations->createField("type", ['type' => 'text', 'collapsed' => 2]);

// example find
$pages->find("template=animal,type=1|2,category=3|4|5");
// example migration
$rockmigrations->createField(Animal::field_type, [
  'type' => 'text',
  'collapsed' => Inputfield::collapsedBlank,
]);

// example find
$pages->find([
  "template" => Animal::tpl,
  Animal::field_type => [Animal::type_cat, Animal::type_dog],
  Animal::field_category => [
    Category::small,
    Category::medium,
    Category::large,
  ],
]);

Or with better syntax highlighting:

Lk4cVZc.png

Imagine you had to answer these questions:

  • where (to which pages or logical parts of your app) does the "type" field belong to?
  • which animal-categories am I listing with my page find operation?
  • what is the collapsed state of my Inputfield?

Which of the two code bases above would you want to manage? ? 

  • Like 3
Link to comment
Share on other sites

1 hour ago, MoritzLost said:

If you're worried about changing field names, you should instead invest the time to find better names for your fields.

Fair point and it's true that in practice I rarely change names, but I just feel more comfortable with (a) them all being declared at the top of the class so they can be changed there if necessary and (b) the IDE recognising the names and prompting, rather than relying on 'magic methods'. Either approach achieves that.

1 hour ago, bernhard said:

Imagine you had to answer these questions:

  • where (to which pages or logical parts of your app) does the "type" field belong to?
  • which animal-categories am I listing with my page find operation?
  • what is the collapsed state of my Inputfield?

Which of the two code bases above would you want to manage?

Really good example. Thanks a lot. Food for thought!

  • Like 2
Link to comment
Share on other sites

This not really a deciding point, but about awareness that constants (as they are always static) are, just like static properties, subject to early binding. A lot of code (even mine, I have to admit) uses self:: to reference class constants and static properties, which is going to cause headache whenever such a class is extended and a constant or static property is changed. To be inheritance safe, any code that references such a value needs to use static:: instead of self:: (or address the property through the object instance).

A short illustration:

<?php

class A {
	const one = 'ONE';
	const two = 'TWO';
	public static $three = 'THREE';
	public static $four = 'FOUR';
	public $five = 'FIVE';
	const six = 'SIX';

	function showMe() {
		echo self::one . PHP_EOL;
		echo static::two . PHP_EOL;
		echo self::$three . PHP_EOL;
		echo static::$four . PHP_EOL;
		echo $this->five . PHP_EOL;
		echo $this::six . PHP_EOL;
	}
}

class B extends A {
	const one = 'UNO';
	const two = 'DUE';
	public static $three = 'TRE';
	public static $four = 'QUATTRO';
	public $five = 'CINQUE';
	const six = 'SEI';
}

$b = new B();
$b->showMe();

That in mind, I guess the decision depends on a few questions:

  • Is the code that is using the value executed often? In that case, constants have a small but perhaps noticeable performance advantage.
  • If not, does my IDE of choice favor one over the other between constants and static properties?
  • May the naming be changed at run time for an individual instance of the class? In that case, the actual value bearers have to be instance properties.

For the later case, I like to use a mixed approach where my initial values are declared as constants (though a static property would be just as good) since they jump the eye:

<?php

class TestClass {

  const CLASS_PREFIX_DEFAULT = 'tpl_';
  
  public function __construct() {
    $this->class_prefix = static::CLASS_PREFIX_DEFAULT;
  }
  //...
}

I try to avoid static access through $this:: (last line in the showMe() method above) as much as possible for my own sanity, as my head wants to make an immediate connection to instance access once it sees $this, and I'll likely end up with expressions like $this::$staticVariable, which to me looks too much like $class->$memberName, yet behaves differently.

  • Like 4
Link to comment
Share on other sites

  • 2 years later...

I'm still using class constant everywhere with great success. Another benefit is that if you remove a field during development your IDE will show you all references that have to be removed. Here I have removed the field "field_eventname" because I don't need it any more. So I removed the constant definition on top of the file and immediately I get proper warnings from the IDE:

aVoQwwc.png

  • Like 2
Link to comment
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
 Share

  • Recently Browsing   0 members

    • No registered users viewing this page.
×
×
  • Create New...