Jump to content

Wowzers!


jgcode

Recommended Posts

Refactoring someone else code the other day and found this little beauty I thought I would share.

Now, building the buttons into a boolean array then converting to an integer is not such a bad thing (I can think of more elegant ways but I digress).

What made me smile was that the developer created a case for every integer although only a few are possible (0,1,2,4,8,16,32,64,128).

That is a lot of work!!

As I wanted to add a 9th button I decided not to follow their lead and add in another 128 cases.

I refactored instead.

I have no problem making fun of someone else's code, as I know I have written bodgies before too and am happy to share those :)

post-10325-126999946935_thumb.png

Link to comment

...a case for every integer although only a few are possible (0,1,2,4,8,16,32,64,128).

Actually, if the booleans aren't latched or if the user is fast enough, any value between 0 and 511 is possible (although I assume the other cases are actually empty).

In any case, it looks like the code comes from an older version of LV (inverted stop condition, B&W VI icon, terminal labels non-transparent) which presumably did not have the event structure, so this code construct would be legitimate.

Link to comment

Try these on for size!

post-415-127005216553_thumb.png throwpc.gif

post-415-127005224263_thumb.png

That little white block on that last one is my 1920x1200 monitor. frusty.gif I'd need about nine of them side by side to see the whole thing.

At least I don't have to refactor, but try and understand and document that! If it were up to me, it would be rewritten from scratch. That may happen yet at some point.

Link to comment

Try these on for size!

post-415-127005216553_thumb.png throwpc.gif

post-415-127005224263_thumb.png

That little white block on that last one is my 1920x1200 monitor. frusty.gif I'd need about nine of them side by side to see the whole thing.

At least I don't have to refactor, but try and understand and document that! If it were up to me, it would be rewritten from scratch. That may happen yet at some point.

Hit the clean up button, I dare you.

Link to comment

Try these on for size!

post-415-127005216553_thumb.png throwpc.gif

post-415-127005224263_thumb.png

That little white block on that last one is my 1920x1200 monitor. frusty.gif I'd need about nine of them side by side to see the whole thing.

At least I don't have to refactor, but try and understand and document that! If it were up to me, it would be rewritten from scratch. That may happen yet at some point.

Butbutbutbut I thought monitor manufacturers are coming out with wider and wider screens just for us LV guys.

Link to comment

Hope your company has a production hall where that monitor fits in!

But honest: they people that write this kind of code, they certainly always tell the management that they work on a very big project. And we stupid guys won't ever show them such impressive things...

Wait, there is this inline subVi scripting tool. I just need to make it work fully recursive and get it launched via QD. Every day I will aks for pay-rise and a larger monitor....

Felix

Link to comment

Maybe the developer used 'Add Case For Every Value' - which would still be a lot of work, but not as much as manually adding them :P

Can you do this for an integer? I think the poor guy must have been adding them manually!! :)

What's inside case 63?

Felix

Nothing. Why do you ask?

Actually, if the booleans aren't latched or if the user is fast enough, any value between 0 and 511 is possible (although I assume the other cases are actually empty).

Faster than 50ms? I doubt it. :) But, yes missing states is a downfall of a polled interface.

It looks like the code comes from an older version of LV (inverted stop condition, B&W VI icon, terminal labels non-transparent) which presumably did not have the event structure, so this code construct would be legitimate.

Regardless of the event structure (this code is currently 7.1, thats how I got it) - Legitimate? Not really, all cases other than (0, 1, 2, 4, 8, 16, 64, 128 etc...) should have fallen under the Default case to do: "Do Nothing".

Try these on for size!

post-415-127005216553_thumb.png throwpc.gif

post-415-127005224263_thumb.png

That little white block on that last one is my 1920x1200 monitor. frusty.gif I'd need about nine of them side by side to see the whole thing.

At least I don't have to refactor, but try and understand and document that! If it were up to me, it would be rewritten from scratch. That may happen yet at some point.

OMG! Look at that nesting!

Good luck with this project ;)

Keep em coming!

Link to comment

Regardless of the event structure (this code is currently 7.1, thats how I got it) - Legitimate? Not really, all cases other than (0, 1, 2, 4, 8, 16, 64, 128 etc...) should have fallen under the Default case to do: "Do Nothing".

I was refering to the basic construct of building the values into an array and converting to a number. I agree that adding a million empty cases is pointless.

Link to comment

I was refering to the basic construct of building the values into an array and converting to a number. I agree that adding a million empty cases is pointless.

Thank goodness for that ;)

I prefer using Search Array (for a TRUE), then that would fix the issue you raised about latched buttons.

Link to comment

As do I. That's the method I usually use when I have similar code (not for polling controls, but there are other cases where you can get an array of booleans).

Cool.

For the UI, its the old... buttons in a cluster, convert the cluster to an array, then search the array for a true, add 1 (so not found = 0), convert to an enum, that is wired to a case structure so your cases are descriptive...

Link to comment

In any case, it looks like the code comes from an older version of LV (inverted stop condition, B&W VI icon, terminal labels non-transparent) which presumably did not have the event structure, so this code construct would be legitimate.

There's no school like the OLD school!

I've been exploring social networks a bit more and I added a review of "A Software Engineering Approach to LabVIEW" to my LinkedIn profile. I commented that some of the techniques shown are a bit dated, but have the advantage of working in any version of LabVIEW at the base version.

A day or two later, I read on the NI community about a call for new/better templates and everyone is talking about events and classes and patterns (bla bla bla); but the currently shipping basic templates could certainly use a face lift.

New users that might be best influenced by well defined patterns may currently only have a Base license and therefore could not learn the fundamentals of typecasting, string/enum conversion or Boolean logic.

Maybe we need some Old School 2.0 templates? Classic Templates Redux?

Link to comment

Hit the clean up button, I dare you.

Well you threw down the gauntlet, so I had to pick it up! Cleanup handled it pretty well, it took maybe 10 seconds. I can't say it's any more usable; in fact I'd say it's worse. Same code now spread out over 5-10x the real estate. Some messes you just can't clean up...

I feel for you, Scott! That looks like all the projects I inherited when I started working here. I can very well understand why everyone else in my group now hate LabVIEW and refuse to use it.

Thanks Crystal.

If you want to play hunt the white block ....... this is the program I inherited a couple of months ago.

That's pretty bad too. Where do these people come from???

Link to comment

OMG! Look at that nesting!

Good luck with this project wink.gif

Yea, and that's just the nesting you can see! And this is one VI out of hundreds.

The original developer recently had to make some changes (I'm not gonna touch it; it's like a house of cards- remove one VI and the thing implodes) and we did a code review of the changes. One of the people in the review who doesn't know LabVIEW said, "Ow! That hurts my eyes!" Tell me about it. I've gone home with headaches after looking at this code.

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
Unfortunately, your content contains terms that we do not allow. Please edit your content to remove the highlighted words below.
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.