He also cautions people to make sure not to use the names of existing system settings, because getOption() will look there if “showNew” isn’t set in the snippet tag in the content.
But variables passed from a snippet tag also show up in snippets as already-set variables, so you could just as well do this:
. . . which would avoid looking up system settings on empty, eliminating any possible problem if someone accidentally uses a variable name that is also a system setting.
Personally, I always use the second method unless I specifically want to set up a global setting (system setting, user setting, clientConfig, etc.) and want snippets to be able to override that setting.
You could definitely do that. It would protect against collisions with settings and would be slightly faster.
On the other side, it creates the possibility of a PHP warning if the property has been removed or mistyped in a snippet tag, since in that case $showNew wouldn’t exist.
I just had a CMP crash from something similar. Any displayed error will usually crash code that expects a JSON response. You could argue that setting error_reporting(0) would solve that, but I like to set it to E_ALL during development.
Also, using the local variables makes PhpStorm yell at me about using undefined variables unless I stop and create a PhpDoc comment telling it to shut up.
I have a live template in PhpStorm for creating the getOption() call, so I generally prefer doing that. YMMV.
Right, but you have five lines of code, where getOption() could do it in one. OTOH, it will avoid the function call and save a little time (though the function is part of $modx, and already loaded, so it’s probably just a few milliseconds).
What I don’t like about doing it that way is that there’s no way to know if the variable is actually set or not without adding more code. Not knowing can lead to some confusing errors, and debugging that could be avoided.
The final (optional) argument to getOption() gives you control over what happens if the variable is set but empty. If that argument is false, getOption() will use the actual value rather than the default, unlike your code. This would be useful if you wanted to be able to send an empty string in the property and have it used as is.
I used to use isset(), and empty() a lot, but it it often resulted in much longer, more complex code, as well as
more mistakes. Using getOption() saves me time. Again, YMMV.