Idea

Serge A suggested Mar 29, '17 Serge A edited Mar 31, '17

Support optional (missing) values in Variant

This Flexscript code throws an exception, and the following appears in System Console:

exception: FlexScript exception: label notexists does not exist at /0 c: /testlink_instance i: /testlink_associated

There is no exception handling mechanism in Flexscript, and no way to see where the exception was thrown, which is a problem in itself for more complex models.

Even if there were an exception handling mechanism in Flexscript, dealing with missing values is such a common use case, that it deserves to be addressed.

Many statically typed languages moved towards using an optional type (Maybe in Haskell, std::optional in C++, java.util.Optional in Java). This type may contain just a single value or nothing, and usually offers means to check if it's empty or to obtain a value of the contained type in a safe way. For instance, in C++ it is possible to

- use optional<T> in conditional expressions (non empty values are evaluated as true)
- use optional<T>::value_or(default_value) to avoid exceptions

class Variant in Flexsim is supposed to support empty values already (if Variant::type equals VariantType::Null), but other objects don't seem to use this feature, and the public methods of the class don't allow to work productively with null variants.

The proposal is two-fold:

return null Variants rather than throwing an exception in LabelsArray::operator[], SubnodesArray::operator[]; or introduce a new safe element accessor (Variant& LabelsArray::get()) to avoid changing the signature of operator[].
extend the Variant class with methods which allow to check if it is empty or replace a missing value with a default value safely:
```
bool Variant::isnull() const;
Variant Variant::or(const Variant& defaultValue) const;
```

Variant::operator bool() should already work properly.

This is how how it may look for Flexscript users:

Implementation of this proposal would have minimal impact on the Flexscript language itself (no need to introduce new control flow structures). The Variant class needs only a couple of new public methods to support this feature. It would eliminate a huge source of exceptions which are not user-friendly (it's difficult to find the code which triggers them, it's impossible to handle exceptions in Flexscript). The new missing value idiom is consistent with the solutions developed for other languages, and encourages users to be explicit about how to handle missing values.

Old rank() and label() were already safe. New [] operator on .subnodes and .labels throws an exception, which interrupts the script and cannot be recovered from.

flexsim-optional-type-proposal-code.png (2.0 KiB)

flexsim-optional-type-proposal-newcode.png (7.1 KiB)

script.png (5.4 KiB)

Matthew Gillespie commented Mar 29, '17 Serge A edited Mar 31, '17

For 17.0.4, labels["notexists"] will now return a null value and not throw an exception.

subnodes[3] will continue to throw an error it it doesn't exist since you should check the subnodes.length property before putting in an out of bounds value.

subnodes["notexists"] will continue to return a null value and not throw an exception as it did before.

· 3

Serge A commented · Mar 30, 2017 at 07:58 AM

Good. Thank you. Would you also consider .or(default_value) method on Variant? That would help to have more declarative oneliners in the Flexsim GUI fields. Explicit if-else checks require custom code, which is not transparent to the GUI users.

This is an example from my reply to Mischa:

Flexsim GUI oneliner

GUI users can see what data is provided by the oneliner. The oneliner is declarative and easy to reason about.

Custom code in Flexsim GUI

Custom code is opaque to the GUI users. A separate window has to be opened to read or change it. Reasoning about procedural code with a handful of local variables and conditional statements is more difficult for non-programmers.

This is also the reason to avoid exceptions in subnodes[3]. If a conditional statement is required for every node access, then users have to write custom code every time they want to access a subnode, GUI becomes much less useful.

0 ·

Phil BoBo ♦♦ Serge A commented · Mar 30, 2017 at 03:47 PM

You can use the ternary operator ? : to do an if/else comparison in one line:

thing.subnodes.length > 2 ? thing.subnodes[3] : "baz"

A conditional statement isn't required for every node access. It is only required if there is a possibility that the node doesn't exist.

0 ·

Serge A Phil BoBo ♦♦ commented · Mar 31, 2017 at 12:05 PM

> You can use the ternary operator ?

Technically, yes, the ternary operator is a solution, but it's a poor one for non-programmers, and a suboptimal one from the usability point of view.

Readability issues: It often leads to unnecessarily long expressions which have to repeat a subexpression:

(thing.subnodes.length >= 2 ? thing.subnodes[2] : otherthing).length >= 3 ? (thing.subnodes.length >= 2 ? thing.subnodes[2] : otherthing)[3] : "baz"
// instead of 
thing.subnodes[2].or(otherthing).subnodes[3].or("baz")

Longer expressions don't fit the narrow input fields of the GUI dialogs: it's harder to "read" what the model does, and it's harder to spot errors while writing. Condition is often many characters away from the subexpression where it matters. Nested ternary operators in a tiny input field are nearly impossible to decipher. All that pushes the users to use custom code almost every time.

Discoverability issues: Unless the user is already a C programmer (or knows a similar programming language), he or she may not know about the ternary operator. The auto-complete feature can suggest to use .or() method, thus any user can learn about it, but the ternary operator idiom relies on user's skills and discipline.

> A conditional statement isn't required for every node access. It is only required if there is a possibility that the node doesn't exist.

There's always a possibility that a node doesn't exist. If node access can throw exceptions, which cannot be handled by the user, checking subnodes.length every time is imperative.

Likely Scenario. A user creates a Process Flow and it uses .subnodes[3]. He or she runs the model, it works. Later the user changes something else in the model, and then suddenly sees this or similar message:

exception: FlexScript exception: label notexists does not exist at /0 c: /testlink_instance i: /testlink_associated

How the non-technical user can understand what went wrong and where? First, it looks like Flexsim itself is a buggy and unreliable software (it did work perfectly, and then for some cryptic internal reason it doesn't). Second, it may take hours, days, multiple phone calls, email exchanges etc to fix this problem. Third, when the user learns the hard way what the problem was and how much trouble it caused, he or she is likely to always check subnodes.length from now on or fall back to using rank() function.

0 ·

Mischa Spelt commented Mar 29, '17 Phil BoBo edited Mar 30, '17

When using the dot notation, you can already do this:

var bad = model().notexists;  // Throws an exception

var maybeGood = model().notexists?; // Note the question mark
if( maybeGood != nullvar ) {
 // ... 
}

I am not sure what the ".labels" equivalent of this would be. Probably you can do something like call find on the labels node (if it exists).

· 2

Serge A commented · Mar 29, 2017 at 01:09 PM

The point is I don't want the exception to be thrown, because it stops the entire script execution, the error message is not helpful, and there is no way to catch the exception. So the script will never get past the "var bad = model().notexists;" line.

Nice trick with the question mark, it's the direction I'd like Flexsim to be moving to; but it's not sufficiently generic. It doesn't work for labels with white space in their names, and doesn't work for SubnodesArray.

Having a one-line error-prone label/subnode/attribute access syntax is important, because oneliners are the default option everywhere throughout Flexsim interface, and writing scripts with conditional expressions is an "hidden" option for advanced users.

This is immediately readable:

This is not:

Right now the workaround is to use the old rank() and label() functions which don't throw exceptions:

I suppose .subnodes[3] and labels["notexists"] should behave similar to rank() and label() in this case.

1 ·

pflow-bad.png (5.3 KiB)

pflow-good.png (5.9 KiB)

script.png (5.4 KiB)

Phil BoBo ♦♦ Serge A commented · Mar 29, 2017 at 04:35 PM

Yes, .subnodes["notexist"] and labels["notexists"] should behave similar to node() and label() in this case.

This is a bug. They shouldn't be throwing exceptions in this case. Thanks for pointing this out. I'll add a note to the dev list to fix this asap.

1 ·

Idea

Support optional (missing) values in Variant

2 Comments

Write a Comment

Things to know…

Your Opinion Counts

IDEA DETAILS

Related Ideas