jgcode Posted March 31, 2010 Report Share Posted March 31, 2010 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 Quote Link to comment
Francois Normandin Posted March 31, 2010 Report Share Posted March 31, 2010 Nice one! You should have kept this for April 1st and provided a scripting tool as an add-on to jcarmody's RCF Case Structure plugin! Quote Link to comment
Omar Mussa Posted March 31, 2010 Report Share Posted March 31, 2010 That is a lot of work!! 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 Quote Link to comment
Black Pearl Posted March 31, 2010 Report Share Posted March 31, 2010 What's inside case 63? Felix Quote Link to comment
Yair Posted March 31, 2010 Report Share Posted March 31, 2010 ...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. Quote Link to comment
smenjoulet Posted March 31, 2010 Report Share Posted March 31, 2010 Try these on for size! That little white block on that last one is my 1920x1200 monitor. 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. Quote Link to comment
Black Pearl Posted March 31, 2010 Report Share Posted March 31, 2010 And if you convert the stacked sequences in the nicer flat sequences, you propably need to break some walls to line all the monitors... Felix Quote Link to comment
hooovahh Posted March 31, 2010 Report Share Posted March 31, 2010 Try these on for size! That little white block on that last one is my 1920x1200 monitor. 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. Quote Link to comment
xtal Posted March 31, 2010 Report Share Posted March 31, 2010 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. Quote Link to comment
PaulG. Posted March 31, 2010 Report Share Posted March 31, 2010 Try these on for size! That little white block on that last one is my 1920x1200 monitor. 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. Quote Link to comment
LHarris Posted March 31, 2010 Report Share Posted March 31, 2010 (edited) If you want to play hunt the white block ....... this is the program I inherited a couple of months ago. Granted, my screen is only 1024x768 and a lot of that is blank space, but still ..... Edited March 31, 2010 by LHarris Quote Link to comment
Black Pearl Posted March 31, 2010 Report Share Posted March 31, 2010 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 Quote Link to comment
jgcode Posted April 1, 2010 Author Report Share Posted April 1, 2010 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 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! That little white block on that last one is my 1920x1200 monitor. 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! Quote Link to comment
Yair Posted April 1, 2010 Report Share Posted April 1, 2010 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. Quote Link to comment
jgcode Posted April 1, 2010 Author Report Share Posted April 1, 2010 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. Quote Link to comment
Yair Posted April 1, 2010 Report Share Posted April 1, 2010 I prefer using Search Array 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). Quote Link to comment
jgcode Posted April 1, 2010 Author Report Share Posted April 1, 2010 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... Quote Link to comment
Phillip Brooks Posted April 1, 2010 Report Share Posted April 1, 2010 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? Quote Link to comment
jgcode Posted April 1, 2010 Author Report Share Posted April 1, 2010 There's no school like the OLD school! I am having flashbacks to Will Ferrell running naked down the street... Quote Link to comment
Francois Normandin Posted April 1, 2010 Report Share Posted April 1, 2010 What's inside case 63? Felix The content is all in case 42. Quote Link to comment
jgcode Posted April 1, 2010 Author Report Share Posted April 1, 2010 The content is all in case 42. Case 42 contains another Case structure with 128 cases. Quote Link to comment
hooovahh Posted April 1, 2010 Report Share Posted April 1, 2010 I am having flashbacks to Will Ferrell running naked down the street... "WE'RE GOING STREAKING" (caps is appropriate he was yelling) NSFW? Quote Link to comment
smenjoulet Posted April 1, 2010 Report Share Posted April 1, 2010 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??? Quote Link to comment
Cat Posted April 1, 2010 Report Share Posted April 1, 2010 The content is all in case 42. That's the answer, then. Quote Link to comment
smenjoulet Posted April 1, 2010 Report Share Posted April 1, 2010 OMG! Look at that nesting! Good luck with this project 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. Quote Link to comment
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.