Sign in to follow this  
  • entries
    132
  • comments
    99
  • views
    88507

There's always a more elegant solution...

Sign in to follow this  
Driv3MeFar

132 views

Let me begin by saying that if you haven't worked with wxWidgets, I recommend it highly. It's a multi-platform library for all your GUI needs, and I'm very impressed with how easy it is to use while still being very powerful.

So anyway, during work today I was assigned a bug that had to do with the color picking menu not working properly. Turns out it was working fine, the handler for entering text was just registered to handle text entry after the user hit "enter". I thought this would be an easy, one-line fix, I just had to change the message ID the handler was registered for.

Turns out that it wasn't that easy (nothing ever is). You see, once text was entered we were calling another function that updated the text fields with the new value. This had to stay in to update the text fields if the user entered an invalid number. Problem was, in that function we called the SetValue() function for the wxTextControls. This in turn generated a wxTextEntry message (or whatever the actual message ID is), which called our event handler, which called the function to update the text fields...

The real solution here is to call ChangeValue instead of SetValue, which updates the text control without generating a text entry event, but that wasn't implemented in the version of wxWidgets that we use at work, so no go on that front. The first thing that came to mind was to set a global flag in the UpdateTextBoxes function, and to immediately return from the event handler if it was set. It wasn't pretty by any means, but it worked.

After going through the usually-excellent-but-in-this-case-laking documentation, I figured out a (in my mind) much cleaner solution to the whole problem. At first, we were setting the event handler with the EVT_COMMAND macro, which statically registers a handler for the lifetime of the program. I changed this to being set dynamically, via the Connect function. That way, when we are updating the text controls, all I have to do is Disconnect() the handler, make the SetValue() calls, then re-Connect() the handler.

It's not the best solution, but it's a lot cleaner than messing about with global flags and whatnot.

So, the moral of the story is, if the only way you can think of to solve a bug is to hack together some code, think harder. And do more research. There is always a cleaner solution out there somewhere.
Sign in to follow this  


2 Comments


Recommended Comments

Let me try to understand... You're changing the value of a control when you are handling the message which has been sent because the value of the control has changed?

Sure, there is a better way to do that: don't change this damn control value [smile].

I mean, the user it typing something inside, and there is not much reason to change it after that. From a GUI design point of view, if you want to display more textual information based on the user input, you'd better write the information in a label near the text box - that's probably cleaner.

Or maybe I missed something. What is you reason to do so?

Share this comment


Link to comment
There are a few reasons why we need to set the value of the control, and not just used the typed in value. Keep in mind, this is for editing color values. Firstly, if the user enters an RGB value above 255, we have to clamp it, and update the display so it doesn't still hold the incorrect value. Also, editing the R/G/B values will change the H/S/V values, so the text controls for them need to be updated as well.

The way the code was structured (since this was only supposed to happen after the user hit 'enter'), we just have one function that goes through and calls SetValue on all the controls, which is called at the end of the event handler. I didn't want to take this out because any time a change was made, I'd have to determine if the change was made to an R/G/B value or an H/S/V value, and update the others accordingly, and that was fairly ugly.

Also, if the color picker was updated by another means, such as selecting a color on the color wheel, all the controls would have to be updated, which would result in the handler being called at least six times (once for each control that was updated), and things got fairly unresponsive quickly. Thus, I just turned off event handling when changing the text control values.

Share this comment


Link to comment

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now