A Thing of Nightmares. Recommendations when reworking abhorrent code?

[etvg]

Hello all,

I was sifting through a code I have been working with and came across this subVI. There is a nice description of what the code does, and some notes sprinkled throughout; however, I can’t help but wonder why something of high importance was written in this manner. They must have been a masochist to stand looking at this code.. Unless, of course, that's why it was banished into the realm of subVIs where they could pretend it wasn't there..

At any rate, I figured I'd post it here so people could enjoy what I am working with and possibly send any tips my way for reworking something which is a complete mess.

Cheers

[Intaris]

I'd love to have a look, any chance of a version 2015 or earlier?

[altenbach]

Typical text programmer (not understanding the concepts of dataflow): Declaring all "variables" by placing all controls disconnected on the left, then read/write exclusively via local variables in the main code. 😞

Current Revision=1324 tells you that this thing has been around for a very long time (another clue is the "Case Constant (See Context Help)" which dates it before 7.1. I feel your pain.

[wiebe@CARYA]

At least it's documented .

[RavensFan]

My first steps:

1.  Make a copy and stash it away so you have something to refer back to.  (especially if notes get moved away from their home because of the next step.)

2.  Run block diagram cleanup.  There are a lot of hidden wires, backwards wires, and wires coming out of the tops and bottoms of functions and variables making it harder to read.  I ran BDC and it seemed to help.

3.  Replace any stacked sequence structures with flat sequence structures.

4.  There are a LOT of wires on shift registers.  See if you can identify what each what is supposed to be and label them.

There are many 1 iteration while loops that seem to be just to hold a previous value in a shift register.  Consider replacing them with feedback nodes.

5.  Look for areas where you can create subVI's.

6.  There are lots of places where similar wires could be put into clusters to minimize wiring.  Might be necessary to do item #5.

7.  Eliminate many or all of those I32 bullets.  Many of them are there to just take an I32 and make it ......   an I32 wire!

8.  There are several places that seem to use several compound arithmetic nodes to divide and subtract values.  Consider combining them into a tiny subVI that could be used in many places.

9.  Remove any local variables when the wire that contains that data is already on the diagram.

10.  ........

[Ben]

Well if you really want to fix it...

Start with the inner most structures and do a diagram clean-up and work your way out. Do not expect but at least it is a start.

Put labels on all of those shift registers just to get started. 

Create cluster with the same data that is being passed around and put it in another shift register.

Start with one of the wires from one SR and switch over to using the data in your cluster.

AS you are working thorugh it start figuring were data is being passed through a srcuture without changes.

Create sub-VIs that accept the cluster does the work and update the cluster when done.

After a few days it may make more sense.

Ben

[wiebe@CARYA]

All kidding aside, it's bad. I've seen worse, but 8/10 on the bad scale.

Start with changing all the tiny things that you know for sure make no difference. For instance, what does that hidden Dummy indicator do? Apparently nothing, unless VI Server is used in other VIs (the horror!). I'd say it's an old debugging artifact...

There are tons of these tiny snippets that only confuse things. Like the compare at the left. If Function = Sine, TC = 1.000m, else it's 1.000m... Hmm..

The locals of Frequencies updated are redundant (use a wire). In fact, the entire indicator is redundant. Do label the wire, or you'll lose the context of it's data!

Guess that will only get you so far... I bet 70% of the code can be replaced with OOTB VIs. Trick is to find out what code and which VIs.

Well, at least the apologized.

[Jacobson-ni]

The inner while loop has 23 shift register pairs. As Ben mentioned, I bet you would get a lot of mileage out of bundling some of those together and pushing some of the work into subVIs.

I actually didn't know you could invert the inputs and outputs of the compound arithmetic primitive when it was in add/multiply mode so I did learn something.

[Bob_Schor]

I "inherited" an entire LabVIEW RT Project, written in LabVIEW 7.0 (before DAQmx), with no documentation, and with the original developers long gone.  As it happened, I was also learning LabVIEW (but with significant Real-Time and other programming experience), and at least had a local LabVIEW Guru for a year who had also struggled with simply maintaining this code.

I was in the fortunate position of having something that continued to work (poorly, but few knew just how bad it was!), and could say "Time to write some documentation, re-think the intended behavior, redesign the entire system from scratch, including scrapping the arcane all-in-one data format with at least 10 variations, and using "modern" LabVIEW (2011?).

Final result is fully documented, uses no code from Version 1, has all new data files that allow streaming data to disk as it is being collected, takes data at double the rate of the old code, is much less hardware-intensive (we had two sets of A/D boards in the old code, one in a PXI to acquire data, and the other in the Host PC so we could display the data -- talk about a nightmare!).  Every VI fits on a 15" screen (I think 1024 x 768, maybe a little larger), and (of course) we use LabVIEW Project.

Do you fully understand what this VI needs to do, and what hardware and other sub-VIs it needs to do it?  If so, I'd suggest doing the following:

• Write a (text) document, as detailed as seems relevant, describing what this code needs to accomplish.  Do not worry, at first, about how you are going to do this, just get the ideas of what down.
• Try to see if there are "concepts" that you can exploit.  For example, if you need to design a filter and filter a signal, there are two steps -- Design Filter (one sub-VI) and Filter Signal (a second sub-VI).  Look for hierarchies and reflect them in a hierarchy of sub-VIs.
• Really try to keep all of your Block Diagram small (Small is Beautiful, and also easier to understand).
• Make your code "Self-Documenting".  Give every sub-VI an Icon (I, myself, like 3-lines-of-Text-in-a-Square icons, really easy to build, and can say "Design Filter" or "Do FFT").  Use of colored background can also be useful (in my RT Project, the Host code was in pale yellow, the RT code was in pale Blue, the "common" code (shared by both) was Pink, Tests were pale Green, etc.
• Use Version Control (which I'm sure you are already doing, but being able to "go back to what worked two weeks ago" is such a life-saver!).

Bob Schor

[Bob_Schor]

Here is the NightMare in LabVIEW 2015 ...

BS