Scary Code

[Ray.R]

Just saw (and fixing(?) ) code recently which is not as big as tst's example, maybe half the size...  Full of stacked sequence structures...

--- sigh ---  but that's not so bad...

--- sigh --- 😞

it is FULL of Local variables.  Each and every value on practically every wire is connected to a Local Variable.  Either read or write.

Also saw something that intregued me and I was impressed for a moment until I realized what it meant..

A cluster whose input cluster is a local... I'll show an example (not from the code)...  needless to say my arms dropped when I realized what it meant.  All clusters are in this fashion.

My example is not quite right...  you'd have to replace the indicator with a Local Variable.  All terminals (ctrl or indicator) are located in one frame not connected to anything.

It's the second time I encouter such style..  The authors are from different cities... Many miles apart..  Same style..  interesting... 😮

Message Edited by JoeLabView on 02-05-2008 08:24 PM

[Ben]

Hi Ray,

I would guess the author of that code does not know about typedefs so using that code constructs allow them to change the cluster def without breaking the code. Let me go onto guess that app does not have many or any sub-VI's.

RE: tst "Scary"

We have taken over a number applications that were orignally developed by other LV shops. One of them has been in developemnt for about 11 years and looks very similar. We have been cleaning the code up one VI at a time. The funny part about code like that is that as I wonder around the cubicle farm and peak at the code others are developing, the first thing of their mouth is;

"I did NOT write this!"

Ben

[Ray.R]

Ben,

I wish you were right!  😮

There are 205 sub-vi's all abusing Local Variables extensively..

I'd get in trouble if I showed you the code..  But imagine sub-vi's having 5 layers of Stacked Sequence Structures..  Every layer passing data via Locals.

Average number of Locals PER terminal is 25.

(is it ok to cry?)

[Wayne.C]

JoeLV,

It might look bad for someone with solid colors to cry.  I found some of CC's friends to do the job for you. 

 

[Kevin_Price]

Ok, educate me.  What makes this a bad style exactly?  I've used this kind of construct and don't (yet) see a good reason to abandon

them.  Am I missing something that should be obvious?  Let me describe how I view it:

1. Not a race condition.  The bundle-by-name node requires the local to be read to act as an input before the output is produced.

2. Not merely writing a value back to itself.  At least some of the cluster elements are being updated, maybe all of them.

3. (Can be) much less diagram clutter to use a Read local rather than a diagram cluster constant, especially for very large and complex

clusters.  The tradeoff is the possibility of an extra copy of the cluster when using local variables.  (If I need to do this kind of thing many

times, I'll make a function which simply returns a default cluster constant, and wire that function into bundle-by-name rather than using

a constant or a Read local).

4. Even though I personally almost never use clusters without typedef'ing them, this kind of construct helps ensure type-matching for

non-typedef'ed clusters.  Using a diagram constant could allow them to get out of sync.

-Kevin P.

[Ray.R]

My initial reaction was.. "wow" why didn't I think of that?  - interesting approach.  Well, I should provide more details..

1. Not a race condition.  The bundle-by-name node requires the local to be read to act as an input before the output is produced.

TRUE if it is the only Local used..

2. Not merely writing a value back to itself.  At least some of the cluster elements are being updated, maybe all of them.

TRUE if it is the only Local used.. (don't forget... the code has up to 5 layers of Stacked Sequence Structures, and everywhere the same cluster is used, it is used in this fashion)  😉

Maybe you're right.. maybe a race condition would not occur because it is wired directly in....  I'll have to think about that some more..  (comments anyone?)  😮

3. (Can be) much less diagram clutter to use a Read local rather than a diagram cluster constant, especially for very large and complex

clusters.  The tradeoff is the possibility of an extra copy of the cluster when using local variables.  (If I need to do this kind of thing many

times, I'll make a function which simply returns a default cluster constant, and wire that function into bundle-by-name rather than using

a constant or a Read local).

I run wires at the top of an Event Structure or State Machine.

4. Even though I personally almost never use clusters without typedef'ing them, this kind of construct helps ensure type-matching for

non-typedef'ed clusters.  Using a diagram constant could allow them to get out of sync.

Good idea..

Side note:  The clusters ARE Type Def'ed..!! 😮

Message Edited by JoeLabView on 02-08-2008 01:19 PM

[Gabi1]

---

JoeLabView wrote:

My example is not quite right...  you'd have to replace the indicator with a Local Variable.  All terminals (ctrl or indicator) are located in one frame not connected to anything.

It's the second time I encouter such style..  The authors are from different cities... Many miles apart..  Same style..  interesting... :o

---

My confession: i began programming Lv exactly this way: all varialbes in the first frame of a sequence. in the following sequences, used only locals.
I think this programming way is very similar to the more classical test based programation: one defines variables, then calls them in the different "pages" of the prog. this might also be a reason why LV has such stacked sequences to begin with. It probably also took time for the NI engineers to develop different programming techniques suited for the graphical language.

[RavensFan]

Here is a very compact program someone had written in LV 5.1  (I guess the style rules didn't exist back then????) that exhibits some of the classic program flaws.

1.  Nested structures.  At least 9 deep.  One has 7 layers!

2.  Local Variables

3.  Program runs continuously unless you crash it with abort.

http://forums.ni.com/ni/board/message?board.id=170&view=by_date_ascending&message.id=301803#M301803

Message Edited by Ravens Fan on 02-14-2008 07:15 PM

[altenbach]

If case structures never existed and we only need code in the TRUE case....

... we could use a FOR loop and convert the boolean selector to 0,1 and wire it to the iteration count.

Now the loop will either spin zero or one times, either executing the code or skipping it. 😄

(from http://forums.ni.com/ni/board/message?board.id=170&view=by_date_ascending&message.id=303356#M303356)

Message Edited by altenbach on 02-21-2008 10:05 AM

[smercurio_fc]

I have to admit that's the first time I've ever seen that. Got it hand to the person that's it seems original.