How to replace global variables with properties in TCheckBox?

207 Views Asked by At

I have a Checkbox1 that I would like to customize that when user clicks on Caption (text) of checkbox it doesn't change it's state (checked/unchecked), but only it does when clicked on the actual checkbox square.

Here is current code with 2 global variables, one that says when to Skip changing state and one that remembers current state and makes sure it stay the same after OnClick - because the state is already changed when flow is in OnClick:

var
  gSkipClick:boolean = false;
  gPrevState:boolean;

procedure TForm1.CheckBox1Click(Sender: TObject);
begin
  // Make sure previous state is assigned when Skip
  if gSkipClick then
    Checkbox1.Checked := gPrevState;
end;

procedure TForm1.CheckBox1MouseDown(Sender: TObject; Button: TMouseButton; Shift: TShiftState; X, Y: Integer);
begin
  if (x > 12) then
  begin
    // Click outside checkbox square
    gSkipClick := True; // skip Click
    gPrevState := CheckBox1.Checked; // save current state
  end
  else
    gSkipClick := False;  // enable Click
end;

Now I wanted to do this with 2 new properties in TCheckBox that would replace global variables:

TCheckBox = class(Vcl.StdCtrls.TCustomCheckBox)
    private
      FSkipStateChange:Boolean;
      FPrevState:Boolean;
    protected
    published
      property SkipStateChange:Boolean read FSkipStateChange write FSkipStateChange;
      property PrevState:Boolean read FPrevState write FPrevState;
  end;

procedure TForm1.CheckBox1Click(Sender: TObject);
begin
  // Click outside checkbox square
  If TCheckBox(Checkbox1).SkipStateChange Then
    Checkbox1.Checked := TCheckBox(Checkbox1).PrevState;
end;

procedure TForm1.CheckBox1MouseDown(Sender: TObject; Button: TMouseButton; Shift: TShiftState; X, Y: Integer);
begin
  if (x > 12) then
  begin
    // Click outside checkbox square
    TCheckBox(Checkbox1).SkipStateChange := True;
    TCheckBox(Checkbox1).PrevState := Checkbox1.Checked;
  end
  else
    TCheckBox(Checkbox1).SkipStateChange := False;
end;

And it works, but if I click on Caption and then close form, this error occurs:

Project Project1.exe raised exception class $C0000005 with message 'access violation at 0x004080c5: read of address 0x0000000d'.

The error occurs in procedure TMonitor.Destroy; in System unit:

procedure TMonitor.Destroy;
begin
  if (MonitorSupport <> nil) and (FLockEvent <> nil) then { <-- ERROR ON THIS LINE}
    MonitorSupport.FreeSyncObject(FLockEvent);
  FreeMem(@Self);
end;

What am I doing wrong, why does error occur?

1

There are 1 best solutions below

1
On BEST ANSWER

First the answer to the question asked about the runtime error. Your interposer class needs to be declared before your form class. You have not done that as can be guessed by your need to cast. You write

TCheckBox(Checkbox1)

but if the interposer was declared correctly you could write

CheckBox1

with no cast.

Look again at every code example for the interposer pattern. The interposer class is declared before any class that uses it.

Because the interposer is declared after the form in your code, the streaming mechanism is instantiating the original class and not the interposed class. Hence your casts are invalid and lead to runtime memory corruption.

It is not enough to define a type and cast to it. You must ensure that this type is instantiated. An object's type is determined when it is instantiated.

As a rule you should always check your casts with is or as. Had you done so here the system could have told you what was wrong. Although you should have been asking yourself why you had to cast in the first place. That should have been the warning signal. Remember that you can always suppress compiler errors by casting but doing so does not solve the problem. Casting something to be something it is not does not actually make it so. It just tells a big fat lie to the compiler and eventually it gets its revenge!


For an interposer you should derive from Vcl.StdCtrls.TCheckBox rather than Vcl.StdCtrls.TCustomCheckBox.


Mixing the code between a derived interposer class and the form is very poor design. When you want to do something similar on a different form are you really going to copy the code again? Code like this needs to be self-contained in the control.

On the other hand maybe the logic is specific to the form. In which case it should live there and you would use the standard check box. It fills me with woe that you refer to using global variables. If you need variables associated with a form, add them to the form class.


The commenters who criticise your UI design are correct in my view. Users like predictability. Nobody will ever be upset when a control behaves in the way they expect, in the way that every other similar control in the system behaves. Stop tinkering with standard controls, use them, let the users be able to predict your program's behaviour.