Is there a better way to code this?

[johndoe108]

I am almost positive this is Rube Goldberg code, but can't be sure.

Can someone help me figure this out? ...asking for a friend...

[altenbach]

What is "this" that the code needs to perform? A better way would be not needing all these local variables (where are the terminal? What's in the other event cases? etc.). Even better would be to connect all the inputs of the select functions (the four on the right have no selector wired).

Please attach the entire VI. Does it work correctly?

Why can't you give your terminals intuitive names linked to their functionality?

[aputman]

If this event structure is handling your entire UI, I would suggest that 100ms on the timeout will be noticeable by a user, depending on the timing of the action in relation to the timeout clock.  If you are not using the timeout frame for any specific purpose (we wouldn't know since your code isn't posted), it's best to remove the timeout value and let the event structure run as fast as possible.  This will help keep your UI responsive to user actions.  You also want to keep actions performed within the event structure to a minimum....no long running code inside.

Edit:  Scratch all that.  It's been a long week already and it's only Tuesday. 

[johndoe108]

Thank you both for replying!

I did not post full code because I am trying to update an older version of a keyboard that is used to enter a product code. I am thinking through how to change the old code to meet new demands placed on the program before I make the change 40+ times (40+ buttons on the keyboard), so I only changed the code for letter 'F' and felt like what I coded was a really round about way of doing it (thus rube goldberg).

The old code is kept in keypad.vi.

The slightly modified code (changes only made in the event for the letter 'F', and nowhere else yet) is kept in keypadv2.vi.

The end goal of this program is instead of only allowing a fixed # of characters for the 3rd and 4th displays and forcing the user to enter from left to right, the user can click on each individual box (with a touch screen, no mouse) and enter characters and backspace characters for each individual display in any order, with upto 3 characters allowed for the 3rd display and upto 4 characters allowed for the 4th display (1st and 2nd displays remain fixed).

I appreciate your feedback.

[cbutcher]

Any chance you could post an older version of the code? Using Save for Previous Version. I have 2017 but I'd guess the earlier you choose, the more people can see it.

[johndoe108]

I believe I did version 13.0 for both of these files.

Thank you.

[cbutcher]

Awesome - thank you for that.

I'd start by suggesting that when you have a lot of duplicate code, you'll probably remove some of your problems if you can reduce it.

In your specific case, one way to do that without changing the functionality or framework/structure of your code would be to group all of the letters into the same Event Case (with many triggering events) and then use the Control Reference (available at the left of the Event Structure in that box that you just can't get rid of, no matter how much you might want to) to determine which letter was clicked.

A simple way to do this is with the Label.Text property of the control, which is accessible as a property node for any control (so you don't need to use To More Specific Class).

It's also not obvious what the selector on the F is doing - probably you don't care about the transition back because you're using latching controls, so you should only get an event for the false -> true transition (assuming you don't start with any of them being true). Perhaps someone can clarify this if I'm a little (or a lot) inaccurate.

As to all of the hidden Display booleans, what you really want to know is which box is being typed into, so store that information directly using an Enum (and perhaps a shift register, or similar). Include a "Not Currently Inputting" option or similar, so that you can handle the case that nothing has been selected.

Once you have these two changes, it should be a lot easier to program each case (and it will be one case, not 40+).

[cbutcher]

Here's a partially implemented version (2017) for T and F, for Display 1. I didn't make the enum a typedef to simplify sharing here, but you definitely should (to simplify changing it later).

Be careful when removing event cases, if you delete the button with the case it will of course delete the letter key too (doh!)

I'm not sure what handling you want, but you can do it differently in each case. It may also be true that you don't need all of the local variables (although not sure). You can at least get rid of the confusing Select nodes.

Edit: I removed your global variables in I think the timeout case, so be careful with that if you make edits directly in the attached file!

[johndoe108]

Thank you for all this feedback!

I will work on these elements you pointed out and will followup once I get a working version.

[ikaiser]

Hey mmkay,

I had some fun implementing my own version, find it attached.

I heavily rely on references, and removed almost all intermediate variables. I chose to use the four ID indicators directly to store the string(s), and jump between them after typing/deleting a character. I also limit input to a certain set or characters (000000-0000-AAA-1 to 999999-9999-ZZZ-9999999999).

Please do NOT use my VI in your productive application, as I took many design decisions that will most likely deviate from your original code.