Suggestions for GOOP coding conventions

[jdunham]

When I was trying to figure out the object-oriented and recursive implementation of the Map Class, I found it very difficult to get a handle on what was going on. My eyes are accustomed to traditional LabVIEW development and I couldn't get my head around how the inheritance and the dynamic dispatching were working to make AQ's design functional.

What I found most confusing was that it was very difficult to see which VIs, controls and wires were 'normal' as oppposed to those which used special features of the language like recursion. Once I gained some understanding, I modified the wires and icons to help remind me what was going on with the design.

1. Dynamic Dispatching VIs are marked with a "serrated edge" between the template part of the icon and the lower part.

In the future, LabVIEW should change the way VIs look if they are dynamic dispatch, and if they are reentrant, with some kind of simple border or overlay. Until that time, I plan to use a consistent marking on my own VIs.

2. VIs used recursively have a cyclic arrow at the left side. (they are all dynamic dispatch too). "Recursive" should be in the VI name. Since LabVIEW can tell that your VIs are used recursively, they should also have a built-in marking.

3. Base classes, which generally have no private data and empty methods designed for dynamic dispatch, are colored gray in the icon template. The object's wire is also dark gray. The class name contains "Base". I wouldn't use this convention if the parent class was something that could exist on its own; this is for situations where no parent class objects should exist, and all objects on that wire are supposed to be an inherited class at runtime.

4. Child Classes have a color in the icon template, and a matching wire color, and the same wire pattern as the base class. The name should start the same way the base class name starts with a modifier at the end.

5. Remember that unbundle text uses the wire color, so only use wire colors with reasonable white contrast.

6. Use old-style BD terminals for built-in data types (including the built-in generic object), and newer icon terminals for lvclass terminals.

I would be curious to see whether these ideas help anyone else understand the Map Class code better, and whether you have more suggestions for conventions we can adopt or ideas to suggest to LabVIEW R&D.

Jason

[Aristos Queue]

I do like the suggestion about wire colors for the abstract ancestors. That's useful. But may I suggest a tweak... there are multiple hierarchies that might be used together on the same diagram, and having them all be gray takes us right back to the "all clusters look alike" problem. Perhaps we could say pastel colors should be used for abstract ancestors and dark colors for concrete classes. That way you can have a red hierarchy, a blue hierarchy, etc.

I've discussed elsewhere several times why dynamic dispatch subVIs aren't called out on the diagram by some glyph, or halo, or something. Search those posts out. Summary: When reading the caller diagram, I really don't care about the underlying implementation of the subVI. Whether it is a static dispatch subVI or a dynamic dispatch subVI is irrelevant -- the icon should tell me what the call does, not how it does it. Similar commentary applies to recursion. Why don't we change the name of functions that call DLLs? Is it important that the caller knows that the subVI uses an external library? No. These are implementation details. If a LabVOOP user develops a brand new set of data types for a LV toolkit and gives them to the LabVIEW user to use, the LV user shouldn't have to understand or be distracted by concepts of dynamic dispatch that have no impact on his/her use of the toolkit. Same goes for recursion. Or DLL calls.

(A counter example: When a subVI is an implementation of a LV2-style global, that *is* something that should be noted on the icon. It impacts the caller to know that this VI has global effect, and its value may be affected by other callers.)

QUOTE(jdunham @ Nov 9 2007, 05:00 PM)

6. Use old-style BD terminals for built-in data types (including the built-in generic object), and newer icon terminals for lvclass terminals.

That doesn't make sense to me. Use one style or the other. Several users have told me that when the types are mixed on one diagram, they get confused about what is a node and what is a terminal. I know it has happened to me too. I find the large icons have lots of useful information for non-LVClass types -- the conpane of VI References, the type of refnums, and typedef identification. It's a style choice whether you like large or small, but pick one.

[Justin Goeres]

QUOTE(Aristos Queue @ Nov 10 2007, 05:48 PM)

I've discussed elsewhere several times why dynamic dispatch subVIs aren't called out on the diagram by some glyph, or halo, or something. Search those posts out. Summary: When reading the caller diagram, I really don't care about the underlying implementation of the subVI. Whether it is a static dispatch subVI or a dynamic dispatch subVI is irrelevant -- the icon should tell me what the call does, not how it does it.

I used to feel (pre LV85) more strongly about this particular issue. The problem in LV82 was that if you opened a dynamic dispatch VI from the block diagram it would only open the most ancestral version of it, which frequently doesn't have any useful code in it. If I was going to have to go to the Project window and dig up the particular implementation I wanted to look at, knowing that in advance from a glyph on the icon saved me several clicks, several seconds, and some confusion.

The dialog in LV85 that lets you select a specific implementation mitigates that a lot, in my experience.

[jdunham]

QUOTE(Aristos Queue @ Nov 10 2007, 04:48 PM)

I do like the suggestion about wire colors for the abstract ancestors. That's useful. But may I suggest a tweak... there are multiple hierarchies that might be used together on the same diagram, and having them all be gray takes us right back to the "all clusters look alike" problem. Perhaps we could say pastel colors should be used for abstract ancestors and dark colors for concrete classes. That way you can have a red hierarchy, a blue hierarchy, etc.

Well I was using different wire patterns to show the different clusters, but I agree it could get confusing. The problem with pastels is that the bundle/unbundle nodes use the wire color for the text, and with pastel colors they are unreadable. The other LabVOOP work I had done recently was basically a factory pattern use of classes, and so I have gray for the abstract class and, orange, blue, and green for the three different types of my object, and there aren't many other objects running around those diagrams, and it works very well.

There are so many ways to use lvclasses that I think this will remain a case-by-case design decision.

QUOTE(Aristos Queue @ Nov 10 2007, 04:48 PM)

I've discussed elsewhere several times why dynamic dispatch subVIs aren't called out on the diagram by some glyph, or halo, or something. Search those posts out. Summary: When reading the caller diagram, I really don't care about the underlying implementation of the subVI. Whether it is a static dispatch subVI or a dynamic dispatch subVI is irrelevant -- the icon should tell me what the call does, not how it does it. Similar commentary applies to recursion. Why don't we change the name of functions that call DLLs? Is it important that the caller knows that the subVI uses an external library? No. These are implementation details. If a LabVOOP user develops a brand new set of data types for a

LV

toolkit and gives them to the LabVIEW user to use, the

LV

user shouldn't have to understand or be distracted by concepts of dynamic dispatch that have no impact on his/her use of the toolkit. Same goes for recursion. Or DLL calls.

(A counter example: When a subVI is an implementation of a LV2-style global, that *is* something that should be noted on the icon. It impacts the caller to know that this VI has global effect, and its value may be affected by other callers.)

Actually, I couldn't find those other posts, but I get your drift. I believe we have different purposes here. You are concerned about the API of your class, and how to document it, and I'm glad that you care. I care about the readability of my diagrams, and those of the other developers whose VIs I am code-reviewing. After many years of LabVIEW, I can look at most diagrams, and run the code in my head. If stuff is hidden from my eyes, then I can't see the bugs.

With a dynamic dispatch VI, the VI I'm looking at on the block diagram may or may not be the one which executes in any given iteration. I have to double-click on the VI to figure out whether seeing is believing. Now it's an order of magnitude longer for me to figure out how this VI works (not the subvi, but just how the caller is working and whether it's working). I'm on the prowl for bugs, so if there are many subvis wired to some object's wire, then a visual indication of which ones are static and which are dynamic will cue me to double-click on the ones which have to be opened to get the full story.

I don't think your argument about the end-user holds much water anyway. In the Map Class we worked on, the end-user API did not have any dynamic dispatch VIs. If you expect the user not to know or even understand them, then it's inappropriate to include them in the API without some wrappers, which is what you did. If they need to use a VI which is dynamically dispatched, you have to make it as clear as possible, because honestly it's always going to be more confusing than the use of a static VI.

For example in the Map Class, if you change the key type from string to anything else, you have to make sure the CompareKey.vi function, which is dynamic, still works with your new Key Class, so you have to get all dirty in it. If you have a dynamic VI on a diagram, I don't believe you can be shielded from how it works and still make and debug your VI. (I actually did leave it shielded it with a static wrapper, which I totally regret now).

Recursion is pretty much the same. In your original implementation of Map Class, Map:Insert Pair.vi is not recursive. It calls Node:Node Insert Pair.vi which may or may not dispatch to itself or to Branch:Node Insert Pair.vi. That in turn calls Branch Node:Insert on Left/Right.vi (statically) which then makes a recursive call to Node:Node Insert Pair.vi. All those icons look pretty much the same, and honestly it took me the better part of an hour to figure out what was going on. I actually started with some recursive mouse clicking of my own before I realized I was going in circles. My time was wasted because the block diagram doesn't go to any effort to illustrate these relationships, even though that's it prime directive.

It would have been great if I could see that the first call to Node:Node Insert Pair was not recursive (it initiates the recursion), and the other call is recursive, which could only be accomplished with an overlay. I added a glyph to my icon, and did some renaming so I could at least flag that the situation is confusing and needs close inspection when debugging.

QUOTE(Aristos Queue @ Nov 10 2007, 04:48 PM)

QUOTE(jdunham @ Nov 9 2007, 05:00 PM)

6. Use old-style BD terminals for built-in data types (including the built-in generic object), and newer icon terminals for lvclass terminals.

That doesn't make sense to me. Use one style or the other. Several users have told me that when the types are mixed on one diagram, they get confused about what is a node and what is a terminal. I know it has happened to me too. I find the large icons have lots of useful information for non-LVClass types -- the conpane of VI References, the type of refnums, and typedef identification. It's a style choice whether you like large or small, but pick one.

Anyone who learns to read the G language is going to find out in short order that the block diagram terminals can be displayed with or without icons, and that the two are equivalent. At this point there are several million VIs with the old style terminals, and most new NI examples seem to use the new ones and there are tons of dead trees in Amazon.com's warehouse with pictures of each kind. So any confusion is already out there and I can't fix it.

So then the decision boils down to which view conveys more information, and what conventions lead to the quickest code ingestion. For my part, I am only going to use the icon, which is very eye-catching (too much so, IMO) when it cues the reader that this terminal is something out of the ordinary. So I might use it for an interesting typedef which relates to my application, and probably for most lvclasses. All of my enums are typedefs, but that big icon is not going to add any insight into the code (and doesn't even prove it's a typedef) so I'm going to leave it small. The error cluster is the error cluster, and using an icon that is enormous is not going to help me understand that. But the lvclass icons really pop, and it was helpful for me to easily differentiate them on the diagram using the icon view.

You suggest that I pick one style, but by using both I can convey even more information, namely, which terminals are "interesting" and which are not particularly so. I used to think the new icons were pointless but now I am a believer.

Thanks for helping me clarify my views,

Jason

[Aristos Queue]

QUOTE(jdunham @ Nov 11 2007, 08:56 AM)

You suggest that I pick one style, but by using both I can convey even more information, namely, which terminals are "interesting" and which are not particularly so. I used to think the new icons were pointless but now I am a believer.

I meant pick one within any single VI, not necessarily across all VIs. Again, the readability issue when both styles are used on a single block diagram is one that has been mentioned to me by more than one user.

[jdunham]

QUOTE(Aristos Queue @ Nov 11 2007, 04:58 PM)

I meant pick one within any single VI, not necessarily across all VIs. Again, the readability issue when both styles are used on a single block diagram is one that has been mentioned to me by more than one user.

OK, I get it, but I don't think it's a big deal. The big terminals look like icons, which is why I never liked them. That's what people are confused about.

A different idea, which I predict you will not like, is to have LabVIEW always display built-in types with old-style terminals, and always display typedefs and lvclasses as icons, and remove the option. This is close to what I was suggesting before, and by taking away the option, you simplify LabVIEW, and you also display some information which you didn't expose before.

Regarding better visual indication of dynamic dispatch VIs, I spoke with my C++ colleague, and I understand better why the class's user is not supposed to care whether or not a method is dynamically dispatched. However when you look at the definition of a C++ function, you can see that right away whether it is declared with virtual, but in LabVIEW it's a lot harder to see that a certain method is dynamic dispatch. I think the block diagram wire for that object has a funny gray 'sleeve' around it but it took me a while to notice that; it's very subtle. And there is the dotted line around the terminal pane connections (which is very, very helfpul), but I still think there is room for more hinting about the nature of the VI.

Maybe something is needed in the project view, or the help window. I still think it would make life easier for class authors (and code reviewers) to see this behavior at a glance rather than digging around for it.

I also think some kind of display of reentrancy and recursion would be extremely helpful.

Thanks for listening,

Jason

[Aristos Queue]

QUOTE(jdunham @ Nov 12 2007, 06:26 PM)

Regarding better visual indication of dynamic dispatch VIs, I spoke with my C++ colleague, and I understand better why the class's user is not supposed to care whether or not a method is dynamically dispatched.

Now this is something that has become *very* interesting to me lately. It's really bugging me on one recent project that I can't see whether *this* VI is reentrant or not unless I open up the VI Properties. It seems ridiculous to have the entire VI Properties dialog displayed as part of the block diagram itself, but there are some items (UI Thread, Reentrancy) that really should be displayed. Take "Allow Debugging" -- I can tell that based on whether or not the Execution Highlight button appears on the block diagram button bar or not. And the appearance settings ought to be *on* the FP.I'm also not liking that I can't look at an entire list of VIs in some view somewhere and see which ones have a password set and which ones don't.I am on record as opposing decorations in the project tree for these sorts of configuration properties of VIs. The reason I have opposed them is that the set of useful-to-know attributes is basically open ended, and there's only so much space for decorating icons in the project tree (it's 16x16, for goodness sake!). So I've pushed to have the only decorations be things relevant to the item's file status -- missing files, checked in/out of SCC, locked against edits, etc. I might conceed the appropriateness of an asterisk to indicate files with unsaved changes. But not reentrancy or dynamic dispatch or anything else that is an attribute of the file's content and not the file itself. Keeping the project tree focused on file aspects seems correct since the project is fundamentally about organizing files and file relationships.But somewhere there ought to be something for sets of VIs to show other types of attributes as a set. In a user friendly, easy to read manner. I don't have any solutions to suggest in this line of thinking. It's just something that has been bugging me lately.QUOTE(jdunham @ Nov 12 2007, 06:26 PM)

I think the block diagram wire for that object has a funny gray 'sleeve' around it but it took me a while to notice that; it's very subtle.

It's meant to be subtle. In fact, if there's no dynamic output, the gray sleeve won't even be there. It's meant to help you debug the situations where dynamic input does not propagate to dynamic output. (It disappears or turns red at places where the connecting path is broken or gets merged with other code paths). So that's not a good way of identifying dynamic or not. QUOTE

And there is the dotted line around the terminal pane connections (which is very, very helfpul), but I still think there is room for more hinting about the nature of the VI. Maybe something is needed in the project view, or the help window. I still think it would make life easier for class authors (and code reviewers) to see this behavior at a glance rather than digging around for it. I also think some kind of display of reentrancy and recursion would be extremely helpful.

The context help window does seem like a good place to display a lot more of this information. I'll take it under consideration. It isn't perfect, but better than nothing.