Jump to content

Code Comments and Documentation


Jim Kring

Recommended Posts

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:

post-17-0-96815900-1324664653.png

Happy Holidays!

-Jim

Link to comment

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 by Wouter
  • Like 1
Link to comment

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.

  • Like 1
Link to comment

This is a very interesting topic thus far, as it has gotten me thinking a lot! :cool:

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.

post-10325-0-36713400-1324686719.png

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:

post-10325-0-43160900-1316163550.png

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.

index.php?app=core&module=attach&section=attach&attach_rel_module=post&attach_id=6025

post-10325-0-21529900-1324687613.png

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).

post-10325-0-11787800-1324687753_thumb.p

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. :thumbup1:

Link to comment

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]

Link to comment

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.

Link to comment

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.

Link to comment

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.

Link to comment
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.

Link to comment

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.... :rolleyes:

Link to comment
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?

Link to comment

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.

Link to comment
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.

Link to comment

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.

Link to comment

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.

Link to comment

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.

Link to comment

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.

Link to comment

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.

Link to comment
  • 1 year later...

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.

  • Like 1
Link to comment
  • 2 years later...

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:

 

  1. #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.

  2. #<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.

  3. #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.)

  4. #Info for any part of code that needs an explanation of what it does. This is particularly necessary when

    1. Uncommon algorithms are used

    2. Complex regular expressions are used.

    3. Making specific assumptions not clear from code.

  5. #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...
  1. 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.
  2. For higher-level VIs, a paragraph-size explanation of the 'process' (considering the input->process->output paradigm) would should be sufficient.
  3. 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.
  1. 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).
  2. 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.
  3. 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 by dhakkan
Link to comment

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.

Link to comment

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?

Link to comment

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...

Important Information

By using this site, you agree to our Terms of Use.