if ($setting) {}
MODX Staff
IMO consistency is more important; since all properties can be overridden in the tag strings, all values should be treated as strings. Components really should always validate the properties/parameters they depend on or make use of to make sure they are set if required and set proper default values if they are not.
$spfconfig = $scriptProperties; if ($spfconfig['property'] || $spfconfg['property'] == '1' || $spfconfig['property'] == 'true')
MODX Staff
IMO, 1’s and 0’s should always be used for so-called boolean properties, then using empty() is sufficient:
SPForm has so many boolean properties, I think it might be best to "normalize" them all with a foreach() at the beginning of the snippet. Otherwise, I’m going to have to type a ton of code like this:
$spfconfig = $scriptProperties; if ($spfconfig['property'] || $spfconfg['property'] == '1' || $spfconfig['property'] == 'true')
if (!empty($property)) { \\it's set so do something with the non-empty, non-zero $property value }
The strings ’true’ & ’false’ or ’yes’ & ’no’ should be avoided unless you want to do comparison’s like you describe.
MODX Staff
How do I know when someone means ’false’ or ’no’ explicitly if those values are automatically converted to 1’s and 0’s in the core? There is no way to normalize these things, nor do I see any benefit from it. I would much rather provide the values as entered in tags and make the component responsible for validating the values it expects. That’s just the way I see it and think it should be; besides, there is no way to know in the parser what type a property value is other than knowing it is always a string, because there is no requirement that these default properties are populated or that they provide descriptions or options. In fact, the only time that information is used is in the property editing manager interface.
Quote from: OpenGeek at Nov 03, 2008, 06:55 PMAgreed, but there’s no way to stop a user from sending them as parameters. I’d take care of that during the "normalization" process.
The strings ’true’ & ’false’ or ’yes’ & ’no’ should be avoided unless you want to do comparison’s like you describe.
MODX Staff
I’m sorry, maybe I’m being hardheaded here, but I personally want this left to the component developer. Making a component easy to use is not the responsibility of the framework. Providing facilities and flexibility for the component developer to provide this ease of use is for the framework. Considering folks have already chosen dozens of ways to deal with boolean switches in the legacy system, I see nothing but frustration for developers who have their own idea of how to properly represent these boolean properties. The docs for the components have to be available to describe what a property does anyway, so saying lots of users won’t read them, well, that’s their problem IMHO. The description can clarify this.
First, users will see the "Yes/No" options in the properties grid and have no way to see the actual values stored. Non-programmers will send `Yes` and `No` as snippet parameters (actually, a pretty logical thing to do) and some PHP programmers will send `True` and `False`. Worse yet, they’ll work half the time, since `yes` and `true` will evaluate to true -- leading to some real frustration. Yes, the docs can educate them, but lots of users won’t read them.
This is trivial; you can store an unlimited amount of properties per element and casting from boolean to string will take longer than simply using the current serialization method for storing this data.
Second, storing a boolean value as a string in the properties field takes more than three times as much space as it needs to (b:1 versus s:1:"1" -- 7 bytes versus 3). Not to mention the separate issue of having stuff like: s:13:"combo-boolean" in there over and over.
Personally, I think the only types that should be available are string and combo personally, and setProperties() is what is used by the UI. There is no data integrity issue here. All values are strings and the description, type, and options are irrelevant to operation. All that is needed on any property is name and value; the rest is for use to make the property editing interface work period. Adding validation to this operation would severely affect performance; I still maintain that the component itself should be responsible for validating any property strings that need to be treated as a specific type in your code.
Third, there’s a problem with data integrity. There are two ways to get data into the properties field -- using setProperties() or with "create" in the grid. In both cases, people can create a variable that doesn’t match its type. You can create a type combo-boolean that has the string "No" as its value (which may be interpreted as true by the snippet code). You can have a type integer with a value of either i:1 or s:1:"1" and a combo-boolean with a value of either b:1 or s:1:"1" . I’m not sure what kind of trouble that could lead to down the road (if any) but it makes me uneasy to have the value inconsistent and possibly not matching the type.
Now for the suggestions. My first choice, which you probably won’t like, would be to store the booleans as booleans and the integers as integers and everything else as strings. Then, when creating the $scriptProperties array and variable list, convert any snippet parameters sent by the user to their proper type (which we’d know by the type declaration in the properties field) using settype(). If we wanted to really make it bulletproof, we’d convert `true` and `false` and $modx->lexicon[’yes’] and $modx-lexicon[’no’] to the appropriate boolean values. This would normalize the properties without any ambiguity, allow simple if$(property) statements in snippets, and let developers use is_string or is_bool reliably.
This is not trivial to implement but would save space in the db. There would be speed gains in the snippet code itself and snippet developers would know for sure that their booleans would come through as booleans and ints as ints. There’a also be speed losses that you’d be a better judge of than I am in the interface to the snippet and in getProperties and setProperties. The extra code would only execute for non strings. I think it could also cause problems with existing snippet code, but I haven’t really thought that through.
Space in the db is not an issue; it would take much longer to deal with this than simply serializing and unserializing the array to the database. If you notice, if you do an $element->get(’properties’) you will get the raw array and if you do getProperties(), which btw you would probably never call from any code in a component (this is automatically done when you call the process() method), you get a simplified set of name/value pairs.
My second choice -- much easier to implement -- is to store everything as a string and rigidly enforce the data integrity when properties are created. Have booleans restricted to ’0’ and ’1’ and integers to numeric strings. We could have a drop-down list for creating booleans in the grid (or throw an error if the value isn’t ’0’ or ’1’). SetProperties() could convert booleans to ’0’ and ’1’ and integer values to their string equivalents (and maybe throw an error for illegal values). The down side of this scheme is that there’s no help for users who send `yes` (in their native language) as a snippet parameter, some space is wasted in the db, and some int values may need a cast in the snippet code.
Independent of these two suggestions, I’d submit that the type codes in the properties field of the DB could be changed from the current string values to something like:
s or t for textField
a for textArea
b for combo-boolean/yes-no
i for integer
l for list
That should gain us both speed and space in the DB.
Personally, I think the only types that should be available are string and combo personally, and setProperties() is what is used by the UI.
MODX Staff
My understanding was that all options had to come from the property definitions and that dynamic list options were not possible at this time. Once they are, sure, that would be great. I’m just trying to keep it simple to start with so we don’t get stuck with stuff we don’t need.
Quote from: OpenGeek at Nov 05, 2008, 12:24 PM
Personally, I think the only types that should be available are string and combo personally, and setProperties() is what is used by the UI.
And this is where Jason and I disagree.
I see an ability for future options such as ’combo-template’, that would allow you to load a dropdown list of templates...storing the value as an ID number. PHP-side, that’s just an integer. UI-side, it’s much more robust.