Setting TRadioGroup ItemIndex without firing OnClick event

Discussion:

(too old to reply)

Paul E. Schoen

2010-05-21 05:30:17 UTC

I have a form with a RadioGroup component with two choices, but sometimes it
needs to be set programmatically, so I just set the ItemIndex property to 0
or 1 accordingly. However, this seems to trigger the OnClick event which
sets the wrong choice, and it also seems to cause a subsequent click on the
form to trigger the OnClick event. I figure that I might be able to set the
OnClick Event to nil when I set the ItemIndex property, but this does not
seem right.

It also does not seem to work. If I do not set the ItemIndex, everything
seems to work, but the RadioGroup is incorrect.

Well, I got something that seemed to work, but it seems very wrong, and I'm
too tired to think clearly. Here's what I came up with:

//****************************** SetTestType
*********************************
procedure TfmOrt.SetTestType(Mode: TTestMode);
begin
if Mode = NORMAL then
TestTypeSel.ItemIndex := 0
else
TestTypeSel.ItemIndex := 1;
// TestTypeSelClick(self);
// This was superfluous, as cahnging the ItemIndex fired the event
end;

//**************************** TestTypeSelClick
*****************************
procedure TfmOrt.TestTypeSelClick(Sender: TObject);
begin
fmDebug.AddDebugItem( 'SetTestType ' );
if fmSetup.TestMode = NORMAL then begin
fmWaveform.WaveFileName := NORMWAVEFILE;
MaxOps := NormalMaxOps;
OrtUpdate1.OnThreshold := fmCal.Cal.OnThresh;
cbMaxOps.Enabled := True;
MinPickUpCurrent.Visible := FALSE;
MinPUlabel.Visible := FALSE;
fmOrtMeter.Visible := False;
end
else begin
fmWaveform.WaveFileName := MPUWAVEFILE;
MaxOps := 1;
OrtUpdate1.OnThreshold := fmCal.Cal.MPUThresh;
cbMaxOps.Enabled := False;
MinPickUpCurrent.Text := '';
MinPickUpCurrent.Visible := TRUE;
MinPUlabel.Visible := TRUE;
fmOrtMeter.Visible := True;
if fmSetup.MaxOnTime < 10 then
fmSetup.MaxOnTime := 10;
end;
SetMaxOps( MaxOps );
ResetData;
fmDebug.AddDebugItem( 'End SetTestType ' );
end;

Originally I had the SetTestType procedure do what is now done in the
TestTypeSelClick handler, as well as setting the ItemIndex. Now it only sets
the ItemIndex. But it still doesn't work right. I've tried all sorts of
things to no avail. This seems like a simple thing. I'll try a little more,
but I'm about beat. I'll follow up if I find a solution. Any ideas?

Thanks,

Paul

Maarten Wiltink

2010-05-21 08:06:41 UTC

Permalink

"Paul E. Schoen" <***@pstech-inc.com> wrote in message news:PToJn.15447$***@newsfe04.iad...
[...]

Post by Paul E. Schoen
procedure TfmOrt.SetTestType(Mode: TTestMode);
begin
if Mode = NORMAL then
TestTypeSel.ItemIndex := 0
else
TestTypeSel.ItemIndex := 1;
// TestTypeSelClick(self);
// This was superfluous, as cahnging the ItemIndex fired the event
end;

You are taking the radiogroup, which is sort of a visual property, and
wrapping it in a property of your own (TestType, of type TTestMode).
Excellent. Nothing wrong with it.

You get notified whenever the radiogroup's itemindex changes. Just what
you want. Don't fight it.

Post by Paul E. Schoen
//**************************** TestTypeSelClick
*****************************
procedure TfmOrt.TestTypeSelClick(Sender: TObject);
begin
fmDebug.AddDebugItem( 'SetTestType ' );
if fmSetup.TestMode = NORMAL then begin
fmWaveform.WaveFileName := NORMWAVEFILE;
MaxOps := NormalMaxOps;
OrtUpdate1.OnThreshold := fmCal.Cal.OnThresh;
cbMaxOps.Enabled := True;
MinPickUpCurrent.Visible := FALSE;
MinPUlabel.Visible := FALSE;
fmOrtMeter.Visible := False;
end
else begin
fmWaveform.WaveFileName := MPUWAVEFILE;
MaxOps := 1;
OrtUpdate1.OnThreshold := fmCal.Cal.MPUThresh;
cbMaxOps.Enabled := False;
MinPickUpCurrent.Text := '';
MinPickUpCurrent.Visible := TRUE;
MinPUlabel.Visible := TRUE;
fmOrtMeter.Visible := True;
if fmSetup.MaxOnTime < 10 then
fmSetup.MaxOnTime := 10;
end;
SetMaxOps( MaxOps );
ResetData;
fmDebug.AddDebugItem( 'End SetTestType ' );
end;

The TestType property is changed, and Magic Happens. No trying to
change the TestType back, or anything to the radiogroup itself, just
nice outward-pointing resynchronisation of related elements. All nice
and clean.

Post by Paul E. Schoen
Originally I had the SetTestType procedure do what is now done in the
TestTypeSelClick handler, as well as setting the ItemIndex. Now it only
sets the ItemIndex. But it still doesn't work right.

How? There is nothing wrong with the above code.

If you want to delay the reaction to the change in TestType's value, you
could add a flag for that, in a standard Begin/EndUpdate pattern. But I
really see no reason why the above 'doesn't work'.

Groetjes,
Maarten Wiltink

Paul E. Schoen

2010-05-21 08:45:20 UTC

Permalink

Post by Maarten Wiltink
[...]

You are taking the radiogroup, which is sort of a visual property, and
wrapping it in a property of your own (TestType, of type TTestMode).
Excellent. Nothing wrong with it.
You get notified whenever the radiogroup's itemindex changes. Just what
you want. Don't fight it.

How? There is nothing wrong with the above code.
If you want to delay the reaction to the change in TestType's value, you
could add a flag for that, in a standard Begin/EndUpdate pattern. But I
really see no reason why the above 'doesn't work'.

I just discovered that there is a problem when I show another form which
contains an "Analog Meter", which is a freeware component I got from a
download. Here is the header info:

Written 1999 by Hannes Breuer (***@talknet.de)
inspired by Andrey Abakumov (***@novoch.ru)

For some reason, as soon as I set focus back to my main form, the OnClick
event fires.

The component is available here: http://www.thebreuery.com/delphi.html

That is a newer version with extensive updates, but using it didn't change
the problem. I don't have it installed as a component. I just included the
source in my project and I used the Create and Destroy methods:

public
AnalogMeter1: TAnalogMeter;

procedure TfmOrtMeter.FormCreate(Sender: TObject);
begin
AnalogMeter1 := TAnalogMeter.Create(Self);
with AnalogMeter1 do begin
Parent := fmOrtMeter;
Align := alClient; //allows testing resize
Left := 8;
Top := 5;
Width := 262;
Height := 234;
Anchors := [akLeft, akTop, akRight, akBottom];
Color := clBtnFace;
Font.Charset := DEFAULT_CHARSET;
Font.Color := clWindowText;
Font.Height := -11;
Font.Name := 'MS Sans Serif';
Font.Style := [];
ParentColor := False;
Caption := '% FS x 10';
Max := 1000;
TickCount := 21;
LowZone := 20;
HighZone := 80;
OKZoneColor := clLime;
end;
end;

procedure TfmOrtMeter.FormDestroy(Sender: TObject);
begin
AnalogMeter1.Destroy;
end;

procedure TfmOrtMeter.FormShow(Sender: TObject);
begin
if Button1.Caption = 'Large' then
fmOrtMeter.Left := (Screen.Width div 2) + (fmOrt.Width div 2) + 2
else
fmOrtMeter.Left := Screen.Width - fmOrtMeter.Width - 1;
end;

I have a linear bargraph built into my main form, but I wanted a circular
meter for easier readability. This problem may have existed for quite a
while, as I had not tested this particular sequence of events. The problem
occurs only when the application starts in the MinPU mode, which is
ItemIndex[1]. When the event fires, it always reads the ItemIndex as zero,
so it gets set back to NORMAL mode. After that, I can switch back and forth
by selecting either radio button and all is OK, until the next time the
application starts.

Thanks for looking at this.

Paul

Maarten Wiltink

2010-05-21 10:30:56 UTC

Permalink

"Paul E. Schoen" <***@pstech-inc.com> wrote in message news:IKrJn.73962$***@newsfe07.iad...
[...]

Post by Paul E. Schoen
I just discovered that there is a problem when I show another form which
contains an "Analog Meter", which is a freeware component I got from a
For some reason, as soon as I set focus back to my main form, the
OnClick event fires.

This *should* not be a problem, but getting it right can be more than
a little tricky.

Naively, the event fires when the property's value has changed. It is
tempting to make the assumption that this means that you went from one
valid value to another valid value. But that assumption is a little too
strong.

What the event really is, is a notification that you need to update
your related elements - without assuming that they are in 'the other'
state (your property is essentially Boolean). If for example you have
a panel whose visibility should follow the property, don't blindly
toggle its visibility in the event handler. _Set_ its visibility to
the appropriate value instead. Do not make assumptions about the
previous value. Write stateless code. I hope that makes it clear enough
for you.

Post by Paul E. Schoen
[...] The problem occurs only when the application starts in the MinPU
mode, which is ItemIndex[1]. When the event fires, it always reads the
ItemIndex as zero, so it gets set back to NORMAL mode. After that, I
can switch back and forth by selecting either radio button and all is
OK, until the next time the application starts.

How does the application start in MinPU mode, if not by setting the
itemindex to 1? You have shown the property setter; it forwards the
value to the radiogroup, which handles the necessary updates. The getter
should read the value from the radiogroup. There is (well, there should
be) no way for TestType to be MinPU except by the radiogroup having its
second button checked.

If you are 'entering MinPU mode' by carefully duplicating all the
_effects_ of switching to it, please throw away that code NOW. Your
radiogroup comes up in a certain state; call that event handler once
at startup and your application should end up correctly initialised.

(Ideally, the radiogroup should be loaded with itemindex=(-1), it should
be cheap to configure application state for that, and restoring settings
would result in updating related elements exactly once.)

Groetjes,
Maarten Wiltink

Paul E. Schoen

2010-05-21 17:46:55 UTC

Permalink

Post by Maarten Wiltink
How does the application start in MinPU mode, if not by setting the
itemindex to 1? You have shown the property setter; it forwards the
value to the radiogroup, which handles the necessary updates. The getter
should read the value from the radiogroup. There is (well, there should
be) no way for TestType to be MinPU except by the radiogroup having its
second button checked.
If you are 'entering MinPU mode' by carefully duplicating all the
_effects_ of switching to it, please throw away that code NOW. Your
radiogroup comes up in a certain state; call that event handler once
at startup and your application should end up correctly initialised.
(Ideally, the radiogroup should be loaded with itemindex=(-1), it should
be cheap to configure application state for that, and restoring settings
would result in updating related elements exactly once.)

I simply preset the ItemIndex=(-1), and now it works OK. Originally, I had
the SetTestType(Mode) procedure take care of all the changes needed, and the
TestTypeSelClick handler simply read the ItemIndex and called
SetTestType(Mode) with the corresponding parameter. But the SetTestType
procedure also set the ItemIndex, which may have caused a recursive state.

In my old code, I had essentially duplicated the SetTestType procedure in
the event handler, so it did not cause the second firing of the event. My
efforts to streamline my code wound up shooting me in the foot. No good deed
goes unpunished, and the smallest bugs seem to be the toughest to squash.

It seems a bit clearer now. Amazing what can be accomplished after a few
hours sleep, a couple cups of coffee, and help from you guys!

Thanks!

Paul

Paul E. Schoen

2010-05-21 19:15:50 UTC

Permalink

Post by Paul E. Schoen

I simply preset the ItemIndex=(-1), and now it works OK. Originally, I had
the SetTestType(Mode) procedure take care of all the changes needed, and
the TestTypeSelClick handler simply read the ItemIndex and called
SetTestType(Mode) with the corresponding parameter. But the SetTestType
procedure also set the ItemIndex, which may have caused a recursive state.
In my old code, I had essentially duplicated the SetTestType procedure in
the event handler, so it did not cause the second firing of the event. My
efforts to streamline my code wound up shooting me in the foot. No good
deed goes unpunished, and the smallest bugs seem to be the toughest to
squash.
It seems a bit clearer now. Amazing what can be accomplished after a few
hours sleep, a couple cups of coffee, and help from you guys!
Thanks!

I found a couple more things that I needed to do before this worked
properly. My call to the SetTestType routine would not fire the OnClick
event if there was no change in the ItemIndex. So I did this:

procedure TfmOrt.SetTestType(Mode: TTestMode);
begin
TestTypeSel.ItemIndex := -1; //This does not trigger an OnClick event
if Mode = NORMAL then
TestTypeSel.ItemIndex := 0 //But now these are guaranteed to
else
TestTypeSel.ItemIndex := 1;
end;

There is still a lot of ugly code in this project, and I am loathe to clean
it up if it works. It seems like a nightmare whack-a-mole game, where you
squash one bug and two more appear elsewhere. Some of this code dates back
to around 2003, when I was just learning Delphi and OOP. I have a better
grasp now, but still there are concepts I may not fully understand.

This has been a difficult, but highly interesting project. Now it includes
aspects of threads, real-time programming (communications handling) using
either a virtual COM port or direct USB meessaging, as well as database and
report generation. Quite a learning experience. I appreciate all the help
given on this forum. And I can't contribute much more than additional
problems for you guys, but I hope you enjoy the challenges to some extent.

Thanks!

Paul

a***@aol.com

2010-05-21 20:15:18 UTC

Permalink

Paul

I still get the feeling that you are fighting Delphi, and doing things
that Delphi does well (& ought to be doing) if you just give it a bit
of direction.

If I was were you are I'd start with a blank sheet of paper & jot some
new ideas of the overall system & code design. Keep a copy of the old
code so you can revert to it if cleaner, clearer, better code does not
ensue.

A sneaking feeling that one's code is overly complex & not elegant, is
usually correct for anyone who has done some serious coding.

Alan Lloyd

Maarten Wiltink

2010-05-21 22:58:22 UTC

Permalink

[...]

Post by Paul E. Schoen

Post by Paul E. Schoen
It seems a bit clearer now. Amazing what can be accomplished after a
few hours sleep, a couple cups of coffee, and help from you guys!

I am honest enough to give the few hours of sleep most of the credit
there.

Post by Paul E. Schoen
I found a couple more things that I needed to do before this worked
properly. My call to the SetTestType routine would not fire the
procedure TfmOrt.SetTestType(Mode: TTestMode);
begin
TestTypeSel.ItemIndex := -1; //This does not trigger an OnClick event
if Mode = NORMAL then
TestTypeSel.ItemIndex := 0 //But now these are guaranteed to
else
TestTypeSel.ItemIndex := 1;
end;
There is still a lot of ugly code in this project, ...

And there is more now. What you do above is not just ugly, it's wrong.
A property should not fire OnChange if its hasn't changed. Conversely,
manually firing OnChange should be harmless and bring everything in
line with the current property value again. As I said, manually firing
the event at program startup may be required to guarantee a valid
initial state.

Groetjes,
Maarten Wiltink

a***@aol.com

2010-05-21 12:51:46 UTC

Permalink

On 21 May, 06:30, "Paul E. Schoen" <***@pstech-inc.com> wrote:
<snip>

Post by Paul E. Schoen
I figure that I might be able to set the
OnClick Event to nil when I set the ItemIndex property, but this does not
seem right.
<snip>

Whyever not <g>

TestTypeSel.OnClick := nil;
TestTypeSel.ItemIndex := 1;
TestTypeSel.OnClick ;= TestTypeSelClick;

Works for me.

If you need to do other things, like calling the OnClick once then

var
SavedOnClick : TNotifyEvent;

TestTypeSel.OnClick := SavedOnClick;
TestTypeSel.ItemIndex := 1;
SavedOnClick(nil); // or SavedOnClick(TestTypeSel); if you use Sender
TestTypeSel.OnClick ;= TestTypeSelClick;

Alan Lloyd