How to go from simple working code to professional code

[whcirobotics]

Hi I was the programmer for team 1514 last year and we did pretty well. Our robot had autonomous which worked 90% of the times and claw and everything worked but the code was "simple". Simple in the sense that it had a bunch of if statements and such, but it did not have any "professional infrastructure". I was wondering if any experienced Labview programmer might know how to make that transition of making simple code to sophisticated professional code. The coding I do works great, but it looks like a bunch of if statements and you get the point. Any help would be appreciated. Tnx

-Chinmay

[Jeff Pahl]

I get paid to write LabVIEW code at my job, so I guess that makes me a "professional" LabVIEW programmer. And I go out of my way to keep everything "simple". If your code works, simple is not a bad thing.

In fact, I was at a NI Symposium last week, and the discussion around the table during lunch (after the excellent presentation on FIRST by Mark Lawrence, the MN Planning Committee Chair) was about the evils of having a very large monitor when programming in LV. The consensus was that having a smaller monitor and trying to hold to a "if it doesn't all fit on one screen it's too much for one VI" rule tended to make for much more concise and easy to read code.

Back to your question, I think the answer you may be looking for in terms of cleaning up your code to make it "more professional" may be to see if you can combine IF statement trees into CASE structures, or possibly to re-structure your code into a state machine design pattern. It's kind of hard to really answer your question without seeing code.

And remember, if it works, and is readable, then however you are doing it is "not wrong".

^ What Jeff said. Keep it simple. Simplicity leaves less room for failure. It also makes things a lot easier for your drivers. 3 buttons are easier to do than 15, and a controller is easier to use than a Power Glove. We know. We've tried it. :|

[tsaksa]

Originally Posted by Jeff Pahl

Back to your question, I think the answer you may be looking for in terms of cleaning up your code to make it "more professional" may be to see if you can combine IF statement trees into CASE structures, or possibly to re-structure your code into a state machine design pattern. It's kind of hard to really answer your question without seeing code.

And remember, if it works, and is readable, then however you are doing it is "not wrong".

This is very good advice. I also program in Labview at work, and it is quite common for me to make some assumptions at the start of a project that may not turn out to be correct, or perhaps some of the requirements get changed over time by the customer. I then end up with code that uses a structure that is not the best fit for what the code really needs to do. At this point it is a good idea to choose the correct design pattern from the Labview examples, and re-do the program mostly from scratch.

This may seem like a bad idea that will waste a lot of time, but it really does not have to. You will always find some key sections to cut and paste. And if you broke complex operations up into sub VIs you probably do not need to touch those. Yes some of the code will need to be redone, but you might be surprised how quickly the work goes the second time through and how much cleaner it looks given what you learned from the first pass. I am sometimes amazed how much work some people will do to avoid starting over even when it is clearly what is needed.

Also, good code has a lot of useful comments. Not trivial comments that basically tell a good programmer the things that they can quickly infer directly from the code and perhaps some well chosen variable names. But more detailed information that describes what you are really doing, why, and how.

Example: This FOR loop sets the motor speed for all four drive motors. Motors 1 and 2 are connected to PWM output 1 and drive the left side of the robot and PWM output 2 controls motors 3 and 4 which are on the right side. All motors are wired such that an output of 254 corresponds to full forward, an output of 127 is neutral, and 0 is full reverse.

Comment 2. The output values to control each motor are calculated in this frame from the analog input values that are .......

You get the idea.

[SuperS_5]

There are also a couple of resources that i forgot to mention; the NI and LAVA.org forums. NI has some really good articles and webcasts that help us in the office, and we program fort a living. The LAVAG code repository has some good examples of clean coding techniques.

If you post an example of your code that you don't think is not good, then maybe we can give some pointers directly related your code.


Please excuse my brevity and typos, these posts are from my phone.

[Greg McKaskle]

It isn't exactly analogous, but it is sort of like you are playing the piano, able to carry the melody, but you want to improve, play the other parts, and generally make better music.

With that analogy, part of the answer is obvious. Practice, practice, practice. Or in this case, spend time working on it, thinking about it, etc. You can study other code, ask for feedback on yours, and ask questions about the topic in general.

I assume that you are posting because you already have some piece of code that you feel should have come out better, but you don't really know how else to do it. Try posting the problem statement and your solution. If you already know of alternatives, describe or implement them too, and let us answer some of your questions or comment on the tradeoffs.

To echo comments from others. Simple can actually be a high compliment in engineering and design terms. True, it can also be a criticism, depending on how it is stated. But simple and working is always preferable to sophisticated and buggy.

Greg McKaskle

[PhilBot]

I agree with most of the comments here, but I'm very surprized that noone has mentioned SubVIs (OK, sorry, a small mention .

If you have a large program that is starting to fill the screen, the BEST way that I know to simplify it is to start thinking of which parts could be best moved into a SubVI.

I beleive that moving from one large program into a programming structure that uses smaller, smarter components is the KEY step which differentiates a "rookie" programmer from a "professional" programmer.

The main reason is that creating good SubVis, or Subroutines, or Libraries requires a certain abount of forethought and planning. Developing your own set of "rules" for this is a real growth experience.

But... for somone just getting started, there are probably two easy ways to know when to create a SubVI:

1) Do you find yourself cutting and pasting code? If so, then STOP! and create a SubVI to embody the function that you are duplicating. Figure out what is common in every copy you were creating and what was changing. Put the common parts in the SubVI, and make the changing parts inputs to the SubVI. LabVIEW makes it very easy to create SubVIs. Just rubber-band ther parts that don't change and select... Edit/Create SubVI. Once you have one, personalize it with your own custom Icon and don't forget to add a description.

2) When you find yourself making one section of your program more and more complicated, to the point where you are moving things around to make space for it..... STOP!, and put that functionality into a SubVI. This way the overall program can stay simple, and the complexity can be hidden in the SubVi .. Then when you need to change the complax part, just drill into the SubVI.

Phil.

[Boydean]

I have little experience with Labview but this subject interests me, and a lot of this advice can be applied to other areas of software development.

Echo what is being said above. Simplify. If something is too complex to explain in plain english then you need to break it down more (more SubVis).

Your code (or blocks) should be explicit as possible. Variables should state what its used for in plain english. Remember that code is read more then it is written – so take the time to actually type "map" and not something like "mp". You ain't special for naming something other people won't understand.

Functions and methods (SubVis) should state what they do in plain english. Use verbs and nouns if the function does something. Example "set_motor_speed" vs "motor_speed".

Comments. Abusive commenting is abusive to the reader. No one wants to read an entire paragraph of comments in the middle of some code. Changing focus from reading coding to interrupting comments with code is annoying. If your code is explicit enough then comments should only be used to explain why you are doing something.

Bad:

// Set motor speed to 0.5.
set_motor_speed(0.5);

Good:

// Anything greater then 0.5 will destroy our arm. Monkeys are awesome. – @brandondean Nov 17
set_motor_speed(0.5);

EDIT: Global variables are bad. Don't use them. Although, if I recall correctly with Labview this is unavoidable...I think.

[Greg McKaskle]

Originally Posted by Boydean

Global variables are bad. Don't use them. Although, if I recall correctly with Labview this is unavoidable...I think.

I agree with everything in the post, except for the global part. I'm not going to claim globals are the best overall mechanism for sharing data, but they have their place, they are simple to understand, and that makes them a reasonable place to start out. This is why they are used in the LabVIEW template code.

Just to be clear, LV has many other data sharing mechanisms, and it is not necessary to use globals. If you feel that you are ready for it, read up on queues, notifiers, rendezvous, occurrences, or perhaps the RT FIFO. Finally, the sharing I personally use the most often is the functional global -- a custom subvi critical section wrapper with just the read and write operations I need. You may be tempted to use events because you have heard about them elsewhere, but since they are primarily for UI operations, they are not appropriate for RT, and the notifier or FIFO is the equivalent choice.

Greg McKaskle

[Ether]

Originally Posted by Boydean

EDIT: Global variables are bad. Don't use them.

I realize that statement was made in the context of FRC and LabVIEW, but in case a reader might think it is a universal principle it should be noted that there are contexts in which it is not true.

Global variables are often indispensable for software for high-volume automotive applications with penny-pinching hardware. If managed meticulously, they can be essential for a successful product.

[whcirobotics]

Originally Posted by SuperS_5

If you post an example of your code that you don't think is not good, then maybe we can give some pointers directly related your code.

Here is the link to the code, i could not upload through megaupload for some reason so this is through Transfer Big files. https://www.transferbigfiles.com/69e...vGqTl9VG1ePaw2

Any pointers:?

[SuperS_5]

Browsing around, I have to say that I have definitely seen a lot worse code. This is actually not to bad. I don't have a lot of time right now but here are a few quick pointers:

1. Use SubVIs!
   • This is the only suggestion for most of the code, including the Begin.VI. Things would be a lot easier to clean up, and understand at a glance if there were SubVIs with descriptive icons. (I don't necessarily mean pictures, text works well too.)
   • The Teleop.VI is in dire need of subVIs.
   • The Autonomous Independent.VI also could use quite a few subVIs.
     For starters, this is not to bad to read.
     • In this case, I would make a shifter for this VI. A shifter is a typedef'd cluster. It holds all of the important data for that VI. It is replaced in OOP (Object Oriented Programming) by the the Class wire.
     • You would put everything like the various digital inputs though their own VIs. You declare that no one else writes to that specific variable in that shifter now. (This the verbal communication part of teamwork.)
     • Then for the bunch of subVIs that you create to do various things, you pull out the Boolean that it needs.
     • The same would be for the numeric controlling the main case structure. However, I would make this an typedef'd enum, and put that into the shifter, and used the same way.
     • All of this would allow you to reuse a lot of the code that you have duplicated many times.
     • I also see some potential race conditions here. It is also fairly difficult to follow the program flow.
2. Clean up the BD
   • After making your subVIs, clean up the BD. For each new subVI, I recommend using the standard 8x4x4x8 connector pane, with error clusters on the 2 bottom corners.
   • Wire the error clusters, even if your VI doesn't care about it. Typically, I won't use the case structure to disable the whole VI on an error like on many NI examples. Instead I will merge only important errors into the main error stream. Usually, most VIs don't really care about an error upstream. (VIs that command hardware, or write to something are my exemptions.)
     This allows for forcing program flow, and tracing errors.
   • Your use of comments is actually really good. The only problem is that without subVIs or any decorations to help point to what the comment refers to, they become kinda pointless. The is especially true for the teleop.VI.
3. Case structure use is okay
   -You mentioned that you have a lot of case structures, but I don't see anything that I would change in the way you used them. If you used subVIs like above, then you wouldn't even notice that you used quite a few.
   
4. ALL VIs should fit onto a single display without scrolling. The resolution of the display is the lowest common denominator of the programming group. If you have to scroll in any direction, pick only one direction, and only one. In fairly rare situations, some coding requires scrolling left and right. (Code with a lot of property nodes sometimes falls into this category. If you yourself scrolling the DB, make subVIs.

Overall I think this is pretty good code. I would probably expect this from a first year co-op student at work. I don't know if it actually works the way you want it to, but it looks like it just might. This is the most important thing.

The above pointers, would take this code into something I would expect from a more experienced programmer.

[Ether]

Originally Posted by whcirobotics

it did not have any "professional infrastructure" ... you get the point

Not really. What does "professional infrastructure" mean to you?

[SuperS_5]

My simple answer is to program, or do anything for 40 hours a week for 5 years or more, and the natural progression will be to get better our more professional. For LabVIEW, you could start off with some courses like our online videos. Anything formal will be a great kick-start.