More complete error checking for levureAppGet/Set

[macMikey]

Comment on issue

[macMikey]

Are you going to finish this PR or is there more to think about?

[trevordevore]

@macMikey I"m going to finish it up. It was a busy week...

[trevordevore]

@macMikey I started doing some testing and I'm wondering if we really want to throw errors if props don't exist when calling levureAppGet/levureAppSet. It isn't very LiveCode-like, although in some cases it can keep us from making silly mistakes.

An example – the preferences helper has a filenameForPlatform function. It checks to see if there is a default preference file name like this:

put levureAppGet("preferences filename>user>default") into tDefaultPrefName
put levureAppGet("preferences filename>user>macos") into tPrefName
if tPrefName is empty then put tDefaultPrefname into tPrefName

With the new behavior of ensuring that every key is present an error will be thrown if every key in preferences filename>user>default or preferences filename>user>macos is not set. The code has to be written as follows:

if levureAppHasProperty("preferences filename>user>default") then
  put levureAppGet("preferences filename>user>default") into tDefaultPrefName
end if
if levureAppHasProperty("preferences filename>user>macos") then
  put levureAppGet("preferences filename>user>macos") into tPrefName
end if
if tPrefName is empty then put tDefaultPrefname into tPrefName

In addition, the same error checking will happen in both levureAppHasProperty and levureAppGet.

I'm going back and forth on this. On one hand, it does keep one from trying to access a property that doesn't exist. On the other, the code becomes longer and harder to read.

One thought I had was to add a second parameter named pCheckExistence. Default is false but if you pass in true to levureAppGet() or levureAppSet then Levure checks to make sure the prop exists before getting/setting. For people writing helpers you could use the parameter if you want to ensure that the developer is setting properties properly. Or you leave the param out and you can just check the result for empty, such as in the case of the preferences helper.

Thoughts?

[trevordevore]

@macMikey Check out current code to see what implementation of pCheckExistence looks like.

[macMikey]

I've now worked on this message for about 30 minutes and gone back and forth on whether I like it or not. I don't see any harm in it. Maybe someone will like it and use it. On the getter side I think it's less useful than on the setter side, but why not have it for both?

[trevordevore]

I did implement it for both.

[macMikey]

Yes, I saw that. I was just saying that I don't think it's as useful on the getter side. Nice edit, btw, lol. I read the original one (via email), then went to check the code, because I thought you had done it for both, then came in, dropped the comment, and then saw you had edited your comment...very sly...

[trevordevore]

I was thinking that if a helper needed the dev to set a property in app.xml then the getter could check for existence in which case Levure would throw an error if the prop wasn’t present.

[macMikey]

Ah. When I wrote issue 102, I was thinking of making the process more automatic, i.e. the helper wouldn't have to do the checking, rather when levure loads the helper.yml it would look in app.yml to make sure that keys that were mandatory/required/not-null were present, and the legal values, ranges, and possibly even datatypes that were specified for various keys matched the values for those keys in app.yml.

[trevordevore]

That would be a nice addition. I'm guessing that is still a ways off though so checking the existence of properties when using levureAppGet might be a useful solution until then.

[trevordevore]

Fixes #120