LabVIEW
LabVIEW Style Challenge - Error Wires
The question was about style, so I didn't change any functionality. Reasoning:
- Unless otherwise specified (and it isn't here) all errors are equally important. I serialized them all on one error wire and let downstream fall by upstream errors. 'Close Config Data.vi' will close reference also on error.
- Constants now have labels visible.
- Controls have got names where first letter in words are upper case. One exception here seems to be "Error in" and "Error out", but "in" and "out" are in this case specifiers of input and output of the same data called "Error". Had some data been named "Header Info" for instance, their two terminals going in and out would have been named "Header Info in" and "Header Info out".
- Functionality is grouped on three separate horizontal levels: First encountered (Config File stuff) on top, then second encountered (queue stuff) below that, and finally third encountered (event stuff) at the bottom. Each functionality group has its main data going left-to-right in a straight line (config file refnum, queue refnum, and event refnum are those straight lines). Error in/out terminals are alligned up with first functionality group.
- Everything is spaced 8 px apart (Shift-arrow) in all 4 directions.
The above is all default style here at GPower.
Just for the sake of conversation, I'll add my comments inline in red.
@JÞB wrote:
Overall, I would make the following changes to Kevin's code, If I didn't reject the silly construction for reasons previously stated.
- Place the Error wire into the loop on a SR (to propagate upstream errors if Key[] is empty.
Understood. I just chose to allow attempts at the 2nd read even if the 1st errored. Either approach could be "right", depending on the app.
- Place the config file refnum on a SR for the same reason.
Thought about it, but opted for the tunnel because the SR wouldn't make a functional difference. I don't need to propagate value changes from one iteration to the next and I can't get caught with a 0-iteration For Loop. So I stuck with tunnels.
I guess the habit comes from SR's catching my attention more, so I want them to be *necessary*. In this example it's purely a preference, and tunnels are mine.
- Branch the Error wire into the loop into three paths immediately at the Left SR inside node.
- Merge all three errors with a report all errors option
- Terminate the loop on Merged Errors = TRUE
Same comment as before -- I *wanted* to attempt the 2nd read even if the 1st failed
- WIRE the Error SR out from the loop directly to Close Configuration File.
- Remove the case structure it's redundant for all Error consumers in this vi EXCEPT for close configuration file which will attempt to close the file anyway and report the upstream error if one existed. By internally passing a No Error into the File Close and merging errors (report first error option.)
I kinda like the explicit convention of the Error case structure wrapper. I wrapped it around the "read-send-send" loop to avoid the unproductive function calls. I chose *not* to put another one around the file open-close pair b/c I didn't think *that* was important enough to justify nested error case structures.
And yes, if you merge copies of the same error ( reporting all errors) you only get one instance of the error because <Status,Code,Source> is found already and not added to the output again.
I haven't really explored the "report all" option to know that it would filter out redundancies that way. That would *definitely* help clean up some of my uglier error wire routing, as seen in my non-loop version.
Thanks for the discussion, I figure that's a big part of what the thread is for -- revealing the *thinking* behind our habits and styles.
-Kevin P
Conversation in italics sill Android
@Kevin_Price wrote:
Just for the sake of conversation, I'll add my comments inline in red.
@JÞB wrote:
Overall, I would make the following changes to Kevin's code, If I didn't reject the silly construction for reasons previously stated.
- Place the Error wire into the loop on a SR (to propagate upstream errors if Key[] is empty.
Understood. I just chose to allow attempts at the 2nd read even if the 1st errored. Either approach could be "right", depending on the app.
pet peeves: Constants that should be controls and maintenance developers that change those constants into controls without thinking through the autoindexing of the For Loops they are used in
- Place the config file refnum on a SR for the same reason.
Thought about it, but opted for the tunnel because the SR wouldn't make a functional difference. I don't need to propagate value changes from one iteration to the next and I can't get caught with a 0-iteration For Loop.
See above. The next guy will write the 0 iteration loop when Keys[] is wired to the Conn Pane
So I stuck with tunnels.
I guess the habit comes from SR's catching my attention more, so I want them to be *necessary*. In this example it's purely a preference, and tunnels are mine.
- Branch the Error wire into the loop into three paths immediately at the Left SR inside node.
- Merge all three errors with a report all errors option
- Terminate the loop on Merged Errors = TRUE
Same comment as before -- I *wanted* to attempt the 2nd read even if the 1st failed
The first read attempt can only error if the refnum is invalid at the loop input. It cannot become valid on subsequent read attempts. Exit on first error. I could see a case to ignore the errors from either Generate User Event or Enqueue but....I still object to the duplication of the Data stored in the object that the Config refnum references
- WIRE the Error SR out from the loop directly to Close Configuration File.
- Remove the case structure it's redundant for all Error consumers in this vi EXCEPT for close configuration file which will attempt to close the file anyway and report the upstream error if one existed. By internally passing a No Error into the File Close and merging errors (report first error option.)
I kinda like the explicit convention of the Error case structure wrapper. I wrapped it around the "read-send-send" loop to avoid the unproductive function calls.
those functions all have standard error handling behavior. You merely nested case structures.
I chose *not* to put another one around the file open-close pair b/c I didn't think *that* was important enough to justify nested error case structures.
again, the case structure is inside the file open and you should attempt to close the refnum even with error in (sometimes people actually FIX upstream bugs preventing file opening and discover a later bug throws an error)
And yes, if you merge copies of the same error ( reporting all errors) you only get one instance of the error because <Status,Code,Source> is found already and not added to the output again.
I haven't really explored the "report all" option to know that it would filter out redundancies that way. That would *definitely* help clean up some of my uglier error wire routing, as seen in my non-loop version.
Thanks for the discussion, I figure that's a big part of what the thread is for -- revealing the *thinking* behind our habits and styles.
-Kevin P
Overall, I did like your approach better than any of the proposals that contained DUPLICATED CODE which is a royal pet peeve.peeves.
Finally, that Configuration File Refnum probably belongs on a FBN inside a private Action Engine of MyIniConf.lvbib with Open, Read, Write and Close public VIs. Of course that will become a PITA if any value datatype other than String is used because the INI API is Polymorphic. Choosing a file type with Metadata (XML, JSON...) would be a good discussion way out of scope for Error Handling Style and Practice.
A GDevCon presentation should address:
- Standard Error Handling functionality
- The new multiple Errors extension to the Error Cluster.
- The Error Ring Xnode
And how they can work together.
The Latest LabVIEW style guide ignores the second and third features completely.
@Taggart wrote:
This messy construct is completely unnecessary:
As with other 'close' functions, "Close Config Data" will close the reference* even with an incoming error. All you need is this:
* However an incoming error would prevent "Close Config Data" from writing data to the file; but since this code only reads data, that doesn't matter.
@paul_a_cardinale a écrit :
@Taggart wrote:
This messy construct is completely unnecessary:
As with other 'close' functions, "Close Config Data" will close the reference* even with an incoming error. All you need is this:
* However an incoming error would prevent "Close Config Data" from writing data to the file; but since this code only reads data, that doesn't matter.
It's a matter of philosophy.
You say that because you have access to the diagram of the close function.
As a TestStand user, I always make silent cleanups.
