IsHandled should never be set to false

[christophstuber]

The IsHandled parameter should never be set to false, neither directly nor as the result of a condition evaluation or function call.

Setting IsHandled to false can cause issues, when an earlier subscriber set it to true.

This is fine:

IsHandled := true;

This is also fine:

if MyCondition then
  IsHandled := true;

This is obviously bad:

IsHandled := false;

This is also very dangerous:

if not MyCondition then
  IsHandled := false;

So is this:

IsHandled := MyCondition;

[ChrisBlankDe]

this only applies if ishandled is a parameter, not a variable.

procedure AnyThing()
var
  IsHandled: Boolean;
begin
  OnDoSomething(IsHandled);
  if IsHandled then
    exit;
  IsHandled := false; // <-- should not raise a warning
  OnDoAnotherThing(IsHandled)
  if IsHandled then
    exit;
  ...
end;

[christophstuber]

@ChrisBlankDe I agree, the conditions should be:

• Is var parameter of type boolean
• The declaring method is an event subscriber

[fvet]

@Arthurvdv @christophstuber

Our current development pattern is to

• always skip early from the event subscribers in case the IsHandled value was already set to true by another subscriber
• run some custom logic and set IsHandled to true in case of success. > Often we use the function call result

So, below pattern is currently used in lots of places and looks good to us (although hard to include this case in the rule I guess)

If IsHandled then
  exit;

...

IsHandled := IsSuccessFullFunctionCall(params,...);

or

If IsHandled then
  exit;

...

IsSuccessFullFunctionCall(params,..., IsHandled);

If above case can't be supported, we will be forced to refactor to below code instead

If IsHandled then
  exit;

...

If IsSuccessFullFunctionCall(params,...) then
  IsHandled := true;