Updating Actions (Don’t Do This)

By | August 23, 2017

What’s my app doing…

So the other day I wrote the following code in an TAction‘s on update event handler. I didn’t spot the issue immediately but my application at certain times was consuming 100% of a CPU on my laptop.

Procedure TXTDefaultMenus.UpdateActionCapability(Const Capability : TXTFileCapability;
  Const Sender : TObject);

Var
  F: TForm;
  FC : IXTFileCapabilities;
  boolEnabled : Boolean;

Begin
  If Sender Is TAction Then
    Begin
      (Sender As TAction).Enabled := False;
      F := FindActiveForm;
      If Supports(F, IXTFileCapabilities, FC) Then
        (Sender As TAction).Enabled := (Capability In FC.Capabilities);
    End;
End;

When I saw it I kind of knew where the issue was. I initially thought it was related to me calling an interface repeatedly but after a little debugging is became apparent that it was actually the code that sets the Enabled property of the action. Now I usually set function returns to a default setting at the start of the method and then evaluate the funtion and I was doing essentially the same thing here however it became apparent with some testing that setting the Enabled property to False and then True in the same call was the issue. It seems to send the main thread into a very quick loop.

To fix the problem I re-wrote the method as below as the Enabled property only gets set once.

Procedure TXTDefaultMenus.UpdateActionCapability(Const Capability : TXTFileCapability;
  Const Sender : TObject);

Var
  F: TForm;
  FC : IXTFileCapabilities;
  boolEnabled : Boolean;

Begin
  If Sender Is TAction Then
    Begin
      boolEnabled := False;
      F := FindActiveForm;
      If Supports(F, IXTFileCapabilities, FC) Then
        boolEnabled := (Capability In FC.Capabilities);
      (Sender As TAction).Enabled := boolEnabled;
    End;
End;