code improvements

[GregFreeman]

I am just looking for someone to critique my code because I have been teaching myself labview over the last few months. Any advice is appreciated, especially aesthetics to make it easier to follow. I think it is ok as of now but I'm sure someone on here can give me a few ways to make it better.  Also, just some aesthetic questions. If you look at the front panel you will see that there is a boarder around some of the boolean labels and I can't figure out how to get ride of them. Can someone tell me? Also, I can't get the pressure indicator to look like the other ones no matter the type I replace it with. Thanks in advance.

Oh, I didn't attach sub VI's with it im assuming for what im asking they wont be needed but if they are let me know.

[altenbach]

Code looks actually pretty good.

• I would eliminate the two stacked sequences, because they don't serve any purpose. It is irrelevant in which order the code in the two frames execute. The two code fragments could easy execute in parallel, so why serialize them in a forced order?
• Maybe you want to place all the stuff to the right of the event structure inside the timeout case instead. For example, if stop of initiated, you don't really want to execute all that once more.
• You should really show the labels for all your "value property" nodes, else the code is confusing to the naked eye.
• All your value preporty nodes shold be replaced by local variables. They are more efficient (and always show the terminal name!).
• You use mostly modern controls, so why use a classic LED. Pretty ugly!
• You should probably initialize your shift registers.
• There should be a mechanism to limit the amount of data in the shift registers. You'll run out of memory eventually.

This was just a 30 second glance at it. Maybe I have more time later.... 😄

Message Edited by altenbach on 12-19-2008 11:00 AM

[johnsold]

I agree with altenbach.

The boxes around the labels are controlled by a setting in the Preferences (Mac) or Tools >> Options.. (any platform: Front Panel: Use transparent name labels.  You may need to change the setting and then replace the controls.

The Pressure numeric indicator is Modern style while the others are Classic style.

(Nit picky) Some of the wiring could be cleaned up.  Tunnels do not line up exactly with the wires and some have quite a few bends.  (I hate the auto wiring tool).

Lynn 

[RavensFan]

---

johnsold wrote:

(Nit picky) Some of the wiring could be cleaned up.  Tunnels do not line up exactly with the wires and some have quite a few bends.  (I hate the auto wiring tool).

Lynn 

---

Lynn,

I have to disagree with you on this.  When I opened the VI, the wiring looked unusually clean and neat for a new user of LabVIEW.  I didn't see any unnecessary bends in wires, and the tunnels were in pretty good shape (not always exact, but good enough)

I still don't understand why some people say they hate the auto wiring tool.  I have it turned on for my machine, but I really don't see it coming into play much and certainly not in ways that cause excessive wire bends.  I find that the VI's posted on the forum that have excessive wire bends are due to people who are click happy.  When they are trying to join to VI's, they click all over the block diagram to get the wire from point A to point B rather than just clicking the beginning and end and letting the wire take its own path.  I find that occasionally I need to  move some wire segments, or go back and right click clean up wiring.

Perhaps you can enlighten me on why the auto wiring tool is evil and where these wire bends are in the OP's VI.

[GregFreeman]

Altenbach or others: suggestions on how to limit data in shift registers? And thanks to everyone for the compliments and advice. I have been working really hard at getting labview down and it actually got me hired just out of college. I owe these forums a lot of thanks.

[RavensFan]

I would embed the 3 major shift registers into an action engine.

The initialize case of the action engine would intialize an array of default elements in array of whatever size is sufficiently large for your graphs.  It would also hold a value as a pointer to what is the number of data elements entered.

The default case of the action engine would replace array subset with the new data element to be added based on the value of that pointer.  It would return a subset of the array out of it to be sent to the graphs.

In the event the pointer has reached the end of the array, you would rotate the array to throw away the oldest data and add the new data to the end.  Then it would retun the entire aray to be sent to the graphs.

[RobCole]

Some suggestions in addition to Altenbach's:

* Make terminals on the block diagram show their labels (as well as the value nodes)

* Documentation on ALL VIs including the top level VI.

* Elapsed Time VIs can be viewed as Icons to save a lot of room ( right click -> View As Icon).

* Change the boolean text labels on your switches to  say Start and Stop instead of On and Off (if that's appropriate).

I'm happy to see comments in the code, but my personal preference is for more. Never assume code will not be reused or read by someone else.

Code is pretty clean. Good work - I would be happy if all of our rookies (and some of the experienced developers) coded as well.

You can make existing labels transparent by using the paintbrush tool and Transparent as your colour (upper right corner of pallete).

Keep on codin'

     Rob

[johnsold]

Ravens fan,

Mostly the lack of tunnel alignment bothered me and at least part of that is due to the use of several different connector panes on the subVIs.  When I converted the stacked sequences to flat sequences on the way to eliminating them some extra bends appeared, but I guess I cannot blame that on the auto wiring.

Actually I meant auto wire routing.  I have autowiring turned on.  Although that seems to grab the wrong connector pane terminal more often than the right one when placing a node on an existing diagram.  The auto routing tool seems designed to create extra bends in wires.  It will not wire straight to a terminal if some other object is within some minimum pixel spacing of the wire route.  If I could set that minimum to one or maybe even zero, it might help.  Wiring past a multiply to an add for example.

Lynn 

[RobCole]

With autowiring turned on, you can always press <space> before dropping the VI to turn off the autowiring when you drop it. I find this handy when something wants to wire incorrectly.

     Rob

[johnsold]

Thanks, Rob,

I did not know about that one.

Lynn