LabVIEW
LabVIEW Style Challenge - Error Wires
@SteenSchmidt wrote:
Have you in all honesty never seen a Close fail on a busy instrument, leaving that instrument's ID and other resources unavailable for your next use?
No. I haven't. I have never seen one fail because the instrument was busy. But I'll take you at your word that it is a possibility. What I have seen is not closing a VISA ref due to not hitting the code path with the close (usually due to someone pressing the abort button, or a close second is some convoluted logic and/or the close in the wrong case of a case structure or something) and leaving the port open. That definitely prevents you from communicating until you close the VISA session. But capturing the error on the close function doesn't help you in either of those cases.
@SteenSchmidt wrote:
This complexity is real. Remember, we're not necessarily talking about merely closing a reference anymore, but in fact 'Close Instrument B' in its entirety.
What is this we? That's what you are talking about. What I am talking about is closing a serial port reference. If you need to do something else to leave the instrument in a safe state, by all means, do that as a separate action and collect the error information on that (if it fails, the best you can do at that point is probably just alert the user, but hey at least you did that much.) That doesn't mean you need to collect the error from closing the serial port.
@SteenSchmidt wrote:
I'm constantly reminded why stuff around us doesn't work as expected.
Now I understand why so much of the code I inherit is overly engineered.
It only needs to be complex enough to solve the problem at hand. Anything else is just the ego-stroking clever programmer. Simplicity wins the day.
@Intaris wrote:
Any question with the word "Style" in it is always going to be somewhat controversial. I'm so grateful that there are different styles out there. The only styles that are not OK are the ones done out of ignorance. Everything else, as long as the consequences are known, controlled or have been demed safe to ignore, is fine (theoretically at least). Depends on where your focus is.
This right here.
@SteenSchmidt wrote:
@Taggart wrote:
In AF you have the Stop Core method that gets called whenever the actor stops. The actor can be in any state when it stops. The actor private data may contain a lot of references. Many are opened in prelaunch init when the actor starts, but some of them may be transient. So when the actor stops you have no idea if any of the references are open or not, the goal is simply to make sure they are all closed to prevent memory leaks. No one keeps track of which ones are open and only closes those. Common practice is to close them all and ignore the errors. Adding code to check all that and handle the errors from closing things that were possibly never even opened is completely unnecessary and gives the potential of screwing up that logic and leaving the reference open.I don't care much about "common practice". There are plenty of blissfully ignorant programmers out there, that don't think too deep about the
Are saying Allen and Stephen are blissfully ignorant? I might not agree on with them on everything and I'm certainly not a big AF fan and yet I wouldn't call them that. It's not just AF, that is common practice in DQMH and probably every other framework I've seen. You appear to be the outlier here Steen.
And yes to your point there are plenty of blissfully ignorant programmers out there, but I wouldn't lump Allen and Stephen in that group.
@SteenSchmidt wrote:
Again, I must point out that I'm not saying you should double check every bit in your code with even more code. A lot of what I'm talking about you are probably doing all the time yourself, perhaps just not considering it very consciuosly. But I'll kick anybody that walks up to me and says they *always* do a silent close. Never *always* do something, think about the context you are in NOW.
Thinking is definitely required as a programmer. However, I 100% default to every time I open a new reference in an actor or DQMH module, the first thing I do is go to stop core (or the appropriate MHL case) and add a close reference and I default to not wiring the error. It's a habit and it's served me well. I don't waste time thinking about what I need to do, I just know - open a reference, close a reference. I am perfectly fine with that being unconscious.
@SteenSchmidt wrote:
@Intaris wrote:
The only styles that are not OK are the ones done out of ignorance.Those are the only ones I'm talking about here.
Error handling is functionality though, not "style". Style is fashion, something you have creative freedom with. Error handling is either adequate or not. You can consider and then decide that there is no need for any error handling here, it's ok to suppress all these error cases. Perfectly fine. But applying a template to all your error handling then it becomes a style, a fashion, and that will fail in some cases. Not just look weird, but fail.
You act like there is only one definition of failure and one definition of correct behavior. If you are your own little benevolent dictator, then I guess you get to dictate that and maybe it works...
@Taggart wrote:
@SteenSchmidt wrote:
@Intaris wrote:
The only styles that are not OK are the ones done out of ignorance.Those are the only ones I'm talking about here.
Error handling is functionality though, not "style". Style is fashion, something you have creative freedom with. Error handling is either adequate or not. You can consider and then decide that there is no need for any error handling here, it's ok to suppress all these error cases. Perfectly fine. But applying a template to all your error handling then it becomes a style, a fashion, and that will fail in some cases. Not just look weird, but fail.
You act like there is only one definition of failure and one definition of correct behavior. If you are your own little benevolent dictator, then I guess you get to dictate that and maybe it works...
In the case of our example here, if the config file doesn't exist, what should happen?
- should we just use sensible defaults?
- in that case, do we output an error or not? Do we tell the calling code? Does it need to care?
- Do we just output an error?
- in that case, what to do with the queue and notifier? Do we send a sentinel? Do we ignore them and not send anything?
- also what if the file is found but just one key is missing? Do we treat that differently?
You can't say something is adequate or not without looking deeper at the requirements (and more importantly the end goal of the project). Using the defaults and notifying the user seems like a fine strategy for an attended test station, where you are loading the config on startup and an operator has to manually start the test or machine. That strategy might not be so great for a real-time system that is supposed to run headless.
I don't think there is one correct or incorrect answer here.
@Taggart wrote:
@SteenSchmidt wrote:
Have you in all honesty never seen a Close fail on a busy instrument, leaving that instrument's ID and other resources unavailable for your next use?No. I haven't. I have never seen one fail because the instrument was busy. But I'll take you at your word that it is a possibility.
Example: A common cleanup procedure for a SCPI controlled instrument could be to send it a few commands such as disable outputs, reset to known state, and perhaps close (something). SCPI instruments can easily be in an unresponsive state, perhaps because of prior communication failure, protocol misuse, overload of some kind, keyboard presses from the operator, or firmware malfunction. In these cases it would be practical to look at if the instrument actual acknowledges your SCPI commands when shutting it down.
@SteenSchmidt wrote:This complexity is real. Remember, we're not necessarily talking about merely closing a reference anymore, but in fact 'Close Instrument B' in its entirety.
What is this we? That's what you are talking about. What I am talking about is closing a serial port reference.
We can only have a meaningful discussion if we talk about the same thing. I have said a couple of times already: The *only* thing you and I disagree on, is if we should consider what 'Close X' does before we mute its error output. Nothing else.
In the above, "we" are us, or so I assumed. 'Close Instrument B' was put in this thread by SebastienM, and that is not just closing a refnum. It's the cleanup code for Instrument B (since it's in TestStand's Cleanup group), which I assume from experience does more than just close the comms reference. Usually it also puts the instrument in a safe state. I highlighted that, as a prelude to why that in particular would be a step you'd like to monitor error out from. Very different from just closing a queue refnum.
It only needs to be complex enough to solve the problem at hand. Anything else is just the ego-stroking clever programmer. Simplicity wins the day.
I agree 100%. I think somewhere in this thread I even stated that the best features are those not there. Or I say that all the time at least. The last 3 or 4 presentations I've done on software architecture and software quality have contained exactly that statement:
Are saying Allen and Stephen are blissfully ignorant?
What are you on about? I'm simply saying that Stop Core doesn't tell you to mindlessly clean up refnums. How do you make that a jab at its designers? Stop Core's help states: "Defines what the actor does before it stops [...] such as shutting down any processes the actor initiated in its override of the Actor Core method."
All completely in line with what I said. It's very smart to have Stop Core, but it's used for stopping the actor in the appropriate way for each actor. Most would just have their references cleaned out, a few might need more.
However, I 100% default to every time I open a new reference in an actor or DQMH module, the first thing I do is go to stop core (or the appropriate MHL case) and add a close reference and I default to not wiring the error. It's a habit and it's served me well. I don't waste time thinking about what I need to do, I just know - open a reference, close a reference. I am perfectly fine with that being unconscious.
I agree and I do exactly the same. I do consider for each if it's sufficient just to close and forget about it, and usually it is. It's not unconscious, and I'd guess you also consider, but it's probably fast enough to feel automatic. If you have a more active component in play, I'd wager you'd treat that differently though, than just closing its reference - like sending it a stop command or unloading it explicitly?
Here is the code for 'Close Actor Interface' from GPower's Actor Framework. I think it looks a bit like you'd do it?:
You act like there is only one definition of failure and one definition of correct behavior. If you are your own little benevolent dictator, then I guess you get to dictate that and maybe it works...
Are you still on the ball, Sam?
By all means error handling is pretty much binary: You either handle the errors you need to handle, or you don't. If you miss an important message from your shutdown code, it's not just style. Your code has a bug then.
And this is the really important part I can feel: I'm talking about considering, then deciding what to do. As opposed to not considering and then doing a scripted thing every time. I'm not saying *anything* about what you should decide to do. If you consider and decide to just close and then ignore error out, alrighty then. I do that 99% of the time as well. But I do consider the consequences each time, which I'm sure you do as well. How are you and I different there? I don't know how you read all these nefarious things in my statements. I try to be really clear.
In the case of our example here, if the config file doesn't exist, what should happen? [...]
The OP asked for style, that's what I gave - how I'd lead the wires. I stated that as well, that I didn't adjust at all on functionality. The original example probably lacked many things, and as you say, without requirements, nothing can be assumed. Also, with the same requirements, many different implementations could be considered great and sufficient.
I then picked one particular comment from message 37, where SebastienM wrote: "A good cleanup does not depend on previous error and, in my opinion, should never output an error". I don't agree with the text I marked in red. I've tried to explain that my only concern is applying this mindlessly. Because there's a world of difference between closing a queue refnum and doing an instrument cleanup for instance. The former is typically just fire and forget, while the latter requires more consideration. Is your opinion that you can mute any cleanup code? If it's not, then you and I agree 100% on everything you have written in this thread. So what's the fuss about?
