Jim Kring Posted December 23, 2011 Report Share Posted December 23, 2011 Hi All, I love all the new VIs and improvements showing up in OpenG. One thing I noticed about the new String to Character Array VI is that there are no comments in the code. While this VI is pretty straight-forward to understand, I think it's a great opportunity to highlight best practices for commenting code. And, while my own personal preferences for code commenting style might not be everyone's preference, I figured I'd share the level of documentation that I'd probably include: Happy Holidays! -Jim Quote Link to comment
Wouter Posted December 23, 2011 Report Share Posted December 23, 2011 (edited) For this particulary example I disagree with you. Comment is good, but all the comments in this particular example are in my opinion not needed since the code is very straight forward. When coding in C I wouldn't either comment code like "lenVariable = strlen(strVariable) // get the length of the string" In my opinion only parts of code which are not that straightforward, certain parts of a algorithm, should be documented. When all the parts of the algorithm are straightforward, as in this case, the only thing that needs to be documented is what the algorithm exactly does, which is already done in this case. Of course one could discuss about what the exact definition is of straightforward. Edited December 23, 2011 by Wouter 1 Quote Link to comment
Darin Posted December 23, 2011 Report Share Posted December 23, 2011 Wow, I head slapped myself silly after seeing that image, especially since that little piece of code was my modest contribution. I completely agree about OpenG showing best practice and all, I strongly disagree that this level of documentation is best practice. Looking at the original thread, the first suggested code was slightly more complex. I thought I would see if it was faster than the completely obvious method. It turns out no. Had the more complex code been faster, as it often can be, that would have been worth a comment. My usual philosophy is that well-written G is self-documenting, and adding text comments is like writing the letters of the notes on sheet music, unnecessary and redundant. A picture is worth a thousand words, this makes it seem like a picture requires a thousand words. If I were to add a comment, it would be that I benchmarked and the simple code turned out the fastest with quick descriptions of the alternative. If I see code like that, I don't question what it is doing, I usually want to know why. If I comment "slightly faster than byte array method", that could save my future self more time than "looping over characters in the input string". Again I agree with the spirit of Jim's comment, I'd simply look for a better example. I know that I am text-averse on my BD so my code is fertile ground for lessons in commenting. I'd suggest my Trim Whitespace code as needing a few more sticky notes and not this one. 1 Quote Link to comment
jgcode Posted December 24, 2011 Report Share Posted December 24, 2011 This is a very interesting topic thus far, as it has gotten me thinking a lot! I think it's a great opportunity to highlight best practices for commenting code. I agree that OpenG should comment code well and lead the LabVIEW Community with best practices. A lot of the OpenG functions are small and readable, but others aren't or require some background knowledge which should definitely be added. On average, I haven't seen this level of documentation in OpenG VI's yet (no saying we shouldn't start). On more than one occasion whilst editing OpenG code, there has been a lack of comments (or no comments) and I have added comments (as I was figuring stuff out and thought it would be handy for the next developer to know). So in the submission of new VIs, we should really aim to get this correct. Therefore feedback like this (from anyone) for a VI would be really handy during the review, so it could be discussed before release. However I think we should focus on entire topic: Given the current feedback (and other end users please continue) - What should the aim of our documentation be? I (personally) think the the number one purpose should be to help the reader understand (i.e. read) the code. I think this means that we need target a certain LabVIEW level of the end user when we do this. How low (or high) do we set the benchmark? Should we target the lowest common denominator (newbies) and add comments on wires, for loops etc...? Or should we be doing this anyway? Should we show the names of the functions (primitive label) etc...? Do we assume the user will use Context Help if they are unsure about a primitive (or argument(s) to one)? Would all this commenting be annoying for the average user and make it harder to follow? In the past I have been aiming for intermediate end users (like CLDish level) which I assume would be the average user of OpenG. I.e. I assume they know the function String Length and it's behaviour. The BD is not the only place for comments, there is also a VI description in this case. So the VI in question is understandable to me without the suggested documentation and is why I never bought it up as an issue in the review (nor added it at a later date). However the suggested documentation wouldn't hurt - I just think we should be (or start being) consistent across the entire OpenG Libraries. If I were to add a comment, it would be that I benchmarked and the simple code turned out the fastest with quick descriptions of the alternative. If I see code like that, I don't question what it is doing, I usually want to know why. If I comment "slightly faster than byte array method", that could save my future self more time than "looping over characters in the input string". Agreed, that information could be handy in this case e.g. that this implementation was slower: As a side note: Maybe a link to the review would be good (more work to possibly maintain for the OpenG Developer)? - This is all traceable through it's SoureForge artifact tho. I'd suggest my Trim Whitespace code as needing a few more sticky notes and not this one. So in re-reviewing this VI based on your comment: At the time I thought the Trim Whitespace has sufficient documentation as well (I personally added it). The ASCII table was also programmatically created and is documented (this is more for OpenG developers though and is not included in the released package). My usual philosophy is that well-written G is self-documenting, and adding text comments is like writing the letters of the notes on sheet music, unnecessary and redundant. A picture is worth a thousand words, this makes it seem like a picture requires a thousand words. I personally sit slightly on the other side of the fence here - I would prefer a little bit more (clear and concise) documentation, even if it was slightly redundant to the code. However, given this thread I think it raises another question: How (well) do people read LabVIEW Code? For me I find that text is always more understandable (and faster to understand) than LabVIEW code. However, I can quite happily work my way through the code I have posted above and figure out what is happening easily enough. Is this the same for the average user? And should e.g. Trim Whitespace have more documentation - I am now thinking yes. In summary the LabVIEW End User Level we are aiming for and therefore the content/amount of the documentation should definitely be identified, reviewed and standardised for OpenG practices (along with the assumptions we make about everything). This thread is a great place to do that. Quote Link to comment
jgcode Posted December 24, 2011 Report Share Posted December 24, 2011 When coding in C I wouldn't either comment code like "lenVariable = strlen(strVariable) // get the length of the string" This was an interesting point and I had a look at this. Here is the same code in a text-based language: <script type="text/javascript"> // converts a string into an array, where each character is an element in that array function toCharArray(str) { var numChars = str.length; var charArray = new Array(numChars); for (i=0; i<numChars; i++) { charArray[i] = str.substr(i, 1); } return charArray; } </script>[/CODE]The result?I find that the code is self-documenting - if I use good variable names.I have added a description of the what the function does - but to me this is the equivalent of VI documentation (so maybe it should have been copied to the BD).So the documentation is ok if this is how we convert the LabVIEW code in our heads when we read it.I guess one thing that is missing is variable names - we specify inputs and outputs nicely in LabVIEW, but maybe labels on the wire should definitely be used in the example.Note: ToCharArray functionality already exists in js, this was just an example.[CODE] <script type="text/javascript"> var str="OpenG.org"; var toCharArray = str.split(""); </script>[/CODE] Quote Link to comment
Darin Posted December 24, 2011 Report Share Posted December 24, 2011 Two separate issues here. In the Trim Whitespace example, the documentation for the user is excellent IMO. The comments for the code are what I was referring to. Sometimes it is hard to appreciate that certain things are done deliberately, such as Reverse Array on the root diagram, or in ShaunR's version the constants being inside the Loop and not outside. Optimization and obfuscation are often close cousins in LV. As to reading G I suggest some immersion therapy. I find text distracting outside of terminal labels. A good way I find to achieve immersion is to be too lazy to add text comments. Quote Link to comment
Ton Plomp Posted December 24, 2011 Report Share Posted December 24, 2011 One standard that I try to follow is that every time I use a for loop with auto-indexing, I show the label of that for loop and add a 'for each .....' title to the for loop. This has helped me greatly in getting back after a while to the exact 'why is this for-loop auto-indexing' questions. Quote Link to comment
Rolf Kalbermatter Posted December 25, 2011 Report Share Posted December 25, 2011 This was an interesting point and I had a look at this. Here is the same code in a text-based language: <script type="text/javascript"> // converts a string into an array, where each character is an element in that array function toCharArray(str) { var numChars = str.length; var charArray = new Array(numChars); for (i=0; i<numChars; i++) { charArray[i] = str.substr(i, 1); } return charArray; } </script> [/CODE]I find that proper indention helps a lot in understanding text based code more easily. Definitely more than commenting single function calls whose name should give a clue already, despite some strange naming conventions sometimes.Note: But fighting with the formatting in this editor myself, it's quite possible that your original formatting was in fact more clean than what the post finally showed. Automatic coercion to 8 character tab indention is quite a bad indention format.And the analogous in LabVIEW: a well structured diagram layout in LabVIEW with straight wires and a clear dataflow is much more helpful than commenting each primitive node in LabVIEW. It can be useful to comment various blocks containing loops, cases or a group of nodes that does something useful, but starting to comment the diagram down to every LabVIEW primitive is rather overkill IMHO. Quote Link to comment
jgcode Posted December 25, 2011 Report Share Posted December 25, 2011 I find that proper indention helps a lot in understanding text based code more easily... ...Note: But fighting with the formatting in this editor myself, it's quite possible that your original formatting was in fact more clean than what the post finally showed. Automatic coercion to 8 character tab indention is quite a bad indention format. Agreed! Seems something got messed up when I cut and pasted - I was able to edit it easily enough tho. Quote Link to comment
crelf Posted December 27, 2011 Report Share Posted December 27, 2011 ...I figured I'd share the level of documentation that I'd probably include: You don't really comment all of your code to that granular level, right Jim? I mean, it almost looks like you're trying to embed your design document in your code Quote Link to comment
Popular Post Aristos Queue Posted December 28, 2011 Popular Post Report Share Posted December 28, 2011 In general, Darren and I add comments to our diagrams when, during a buddy session, we actually have to explain what's going on to the other. That's our standard measure for when a comment is needed. If the buddy session proceeds without the other person getting lost as we walk through, then the code and the API documentation (i.e. the Context Help) likely suffices as it is. If I were to add a comment, it would be that I benchmarked and the simple code turned out the fastest with quick descriptions of the alternative. My instinct in this case would be to put a Diagram Disable structure down and put the other version that I had tried in the Disabled frame -- then I don't have to describe what I tried, people can look at it. Now, that is only true when the other version of the code is all prims or vi.lib VIs, things that don't cause my source code to have additional dependencies when distributing it. As much as possible, I would still prefer to include the graphical "here's what I tried" and avoid a text description of it. It is both more accurate to say "I tried this" and if someone else says, "But that other way really should, by rights, be faster..." they can benchmark it themselves easily and/or spot any errors I had in my implementation. And if the compiler takes advances, it's easy to flip in the other code.Now, having said all of that, I do tend to include "proof of correctness" comments with complex algorithms, along the lines of "This loop works because earlier in this VI we already guaranteed that there are no duplicates in the array". This is especially true when I'm relying on a less-than-obvious fact of one of the upstream functions. For example, I recently wrote a VI with the comment "This works because the Controls[ ] property of a Front Panel refnum guarantees that the controls are returned in tab order." Most people analyzing my code for correctness wouldn't know that fact, even if it was properly documented (it will be in LV 2012), so it was worthy, IMHO, of a comment. 4 Quote Link to comment
Val Brown Posted December 28, 2011 Report Share Posted December 28, 2011 FWIW, in general I agree with what AQ posted. If you are trying to make your code as clear as possible (for ongoing maintenance, inheritability for others, etc) then definitely run the VIs through a buddy review and document -- in some fashion -- what the buddy had to ask about. It's best IMHO to use as much graphics as possible, rather than just descriptive text, and this would include colored boxes around code sections, arrows, perhaps even a flowchart of the processes involved. It is also a best practice IMHO to include the previous attempts at resolving the requirements using the diagram disable structure. It's the single best way to document (really record for reuse and understanding) the work that was done. Of course all of this presupposes that "ease of access" by others is the primary requirement of your development process's documentation procedures. If on the other hand it is critically important to protect potentially exposed IP then it is equally important to obscure and obfuscate the code as much as possible. Hmmm perhaps someone can come up with a da Vinci like "mirror writing" or "inverse graphics" symbology.... Quote Link to comment
jgcode Posted December 29, 2011 Report Share Posted December 29, 2011 My instinct in this case would be to put a Diagram Disable structure down and put the other version that I had tried in the Disabled frame -- then I don't have to describe what I tried, people can look at it. It is also a best practice IMHO to include the previous attempts at resolving the requirements using the diagram disable structure. I have never seen (or thought of doing) this before reading it here. Do any other developers do this? Quote Link to comment
Jordan Kuehn Posted December 29, 2011 Report Share Posted December 29, 2011 I have never seen (or thought of doing) this before reading it here. Do any other developers do this? I do not. However, I like this idea quite a lot and may start including it. There are times when I switch to a new approach and leave the old one while I test the new implementation, but I'll usually delete the structure when I'm done. Quote Link to comment
jgcode Posted December 29, 2011 Report Share Posted December 29, 2011 There are times when I switch to a new approach and leave the old one while I test the new implementation, but I'll usually delete the structure when I'm done. Yes, I use the disable structure for testing too (as you mentioned because it's easy to switch back if needed). But I have not used it to facilitate discussing/commenting code. Quote Link to comment
Aristos Queue Posted December 29, 2011 Report Share Posted December 29, 2011 I won't use it (diag disable struct) if I'm fixing a bug or something like that -- that's what source code control is for. But if I want to document "this alternative was tried and is less performant", that's often easier to document for the future using a disable structure. Also, sometimes the speedier algorithm is more complex, and if we get a bug in the future, the disable structure makes it easy for me to flip back to the slower but easier to prove right algorithm, so I can tell if the bug is in the algorithm or somewhere else. Quote Link to comment
Darin Posted December 29, 2011 Report Share Posted December 29, 2011 I am hesitant to use the DDS in situations involving optimization. For reasons beknownst to NI it imposes artificial dataflow and the form factor of the different cases seldom lines up giving me awkward looking code. During the trial and error phases, the DDS is invaluable, no need to leave evidence of my mistakes. I do like to use it to hold code used to generate lookup tables though. Of course in this case the G code serves as a comment, a nice graphical comment as nature intended. Quote Link to comment
asbo Posted December 29, 2011 Report Share Posted December 29, 2011 If I'm doing maintenance to code that wasn't mine originally, I'll either leave a dated, initialed comment describing the change, a DDS with the new and old version, or a combination of the two. When developing, I'll often use the DDS as a sort of temporary version control, showing other attempts at functionality, but typically once I get things working satisfactorily I wipe them all out. I agree with Darin's notion that the occasions where old and new code sizes align is rare, so this is part of why I tend not to leave them. Quote Link to comment
Val Brown Posted December 29, 2011 Report Share Posted December 29, 2011 Understood re: the BD size issue but, of course, the possibility is to simply place a sub there, of the older code and so named, so that the size of the icon for the sub isn't larger than the final code of the deployed design. It depends on the purpose of the documentation but having the ability of backtrack and compare previous incarnations without having to back up a revision or two can be helpful, esp when new versions of LV are released. Quote Link to comment
dannyt Posted January 4, 2012 Report Share Posted January 4, 2012 A little late to this, but here is my take on it. IMHO I would never concider the level of comments that Jim put in his example, even if the end user aimed at was for novices. I have two reasons for this. One is at that in real life, with this level of commets I think I would never actually get any LabVIEW written and the second is that I do not think it is actaully going to help Novices, who need to get to a situation where they are comfortable with LabVIEW primitives and the general look and feel of G code. Somebody new to LabVIEW would just quickly read the comments and miss what the actual G-Code is. I am very much with AQ in the idea of peer review, if the person reviewing the code needs an explination then a comment is definitly required. Line labels are good, if lines travel a long way across a screen, sensiable labels for while and for loops can be very helpful as well. Quote Link to comment
jdunham Posted January 7, 2012 Report Share Posted January 7, 2012 I can pile on here. One reason not to over-comment is that then it makes it harder to figure out which comments are important. Sometimes less is more. Admittedly more often code suffers from a dearth of comments than from a surplus. 1 Quote Link to comment
Derek Shpuniarsky Posted January 8, 2013 Report Share Posted January 8, 2013 I don't think the criteria I am posting is different from the general consensus of the previous replies, but it might help others as it helps me to decisively determine where to draw the line... Â I always filter comments based on whether they answer 'what am I doing' or 'how am I doing' something and leave out the 'hows'. The level of detail is based on what questions someone else familiar with the purpose of the SW would ask (the peer review scenario) or similarly, what I will forget about a couple months after finishing it (the SW update scenario). Â Exceptions would be comments answering the 'why' questions that require knowledge contained in a manual the programmer may be unfamiliar with. AQ gave one example of this earlier. Another example would be including documentation about the bits in a product's register when manipulating them. 1 Quote Link to comment
dhakkan Posted December 11, 2015 Report Share Posted December 11, 2015 (edited) Sorry about reviving an old thread. However, am hoping this may help... My 2c follows. Â Â Consider using LabVIEW's bookmarks and subdiagram labels. I'm beginning to use the former as follows: Â #TBD for any part of code that needs to be re-visited for any reason. Goal is, by the end of development cycle, no such bookmarks must remain. #<NameOfDev> for any part of code that needs to be re-visited by the named Developer. Goal is, by the end of development cycle, no such bookmarks must remain. #Rationale for any part of code that needs an explanation of why the implementation is so. (This would include info about trying other approaches as stated in earlier posts.) #Info for any part of code that needs an explanation of what it does. This is particularly necessary when Uncommon algorithms are used Complex regular expressions are used. Making specific assumptions not clear from code. #Reference for any part of code based on reference material available to the developers. Â Rationale: The first two bookmarks above are meant to be on-the-spot reminders for developers to verify or seek additional information when having questions. The remaining bookmarks are meant to aid future developers for easier code maintenance or revisions. The Bookmarks Manager helps with finding the first two prior to release, provided Developers are diligent and consistent enough... Â Subdiagram labels help me a fair bit. I use them initially to jot the implementation approach; then refactor the verbiage as needed following the hashtags above. Â Reg. context help... For low-level VIs, if the inputs, VI name and outputs are not not self-explanatory; there is likely scope for improvement in the names. That said, of late, I've started to document non-default VI execution properties in the context help. (E.g. 'Shared reentrant', execution thread, etc.). It would be great if NI would make context help more powerful by displaying these important non-default properties. Lots of people seem to support this in the idea exchange... I've also started to specify a line in context help if VI does NOT provide 'standard error out' functionality. I don't provide separate 'detailed help' documentation unless contractually bound to do so. For higher-level VIs, a paragraph-size explanation of the 'process' (considering the input->process->output paradigm) would should be sufficient. If more information is warranted than the above two points, then it's better to provide 'detailed help' for those VIs. I used to label wires a LOT. Have toned it down as follows. Attach a label to a 'shift register' outside the sub-diagram, if its initial value is not immediately apparent (e.g. a case that's not displayed). Apply a label with direction in case a wire originates at a point beyond the visible part of the block diagram. E.g. 'PSU Object ->'. If a feedback node is used the direction could be right-to-left, in which case '<- PSU Object' would work. Don't label a wire otherwise. It's easy enough for the Developer to open the context diagram and see its source or sink to determine what it is. I label a constant primarily if it is not obvious from nearby labels. Another example of the obvious: Type-def constant should not need labeling given that its info is visible in the context help.) Edited December 16, 2015 by dhakkan Quote Link to comment
hooovahh Posted December 11, 2015 Report Share Posted December 11, 2015 Yeah I made a thread, and then eventually a utility that attempts to make standard bookmarks easier to add. Â It can be invoked from the tools menu, or with a little work from a quick drop. Â It brings up a window that shows all the standard bookmarks, along with descriptions of each one. Â There is some that could ask for more information too, like a tag that is covering a requirement, which then asks what requirement to put in the comment. Â There is also a scan option for finding all tags already in a project. Â I honestly don't use this all that often, but I made it hoping it would encourage me to use the bookmark manager more. Quote Link to comment
ThomasGutzler Posted December 14, 2015 Report Share Posted December 14, 2015 I really liked the idea of the bookmark manager, but I'm a little disappointed by how slow it is. Â My "small" project has 2600 files (in memory) and the bookmark manager takes around 40s to open. Plus, every time I modify a bookmark it takes another 40s to update. This is often a serious interruption in workflow. I click on a TODO bookmark, do the work, remove the bookmark, save the VI and then I'm sitting there waiting for the bookmarks to update before I can go to the next item. I just can't convince myself to go to the next bookmark BEFORE saving the file I'm currently working on. How do use it? Quote Link to comment
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.