Faster code If/then

Question

I found some code like

procedure UpdateComp(Sender: TObject)
begin
  if Sender = edtA then
  begin
    ...  //to something
  end;
  if Sender = edtB then
  begin
    ...  //to something
  end;
  if Sender = edtC then
  begin
    ...  //to something
  end;
  ...
end;

The if/then in the code is for more than 50 statement Each time is the sender match with a component on the form ones. Is it then logic to change the code like:

procedure UpdateComp(Sender: TObject)
begin
  if Sender = edtA then
  begin
    ...  //to something
  end
  else if Sender = edtB then
  begin
    ...  //to something
  end
  else if Sender = edtC then
  begin
    ...  //to something
  end;
  ...
end;

Or I am not correct to change this way?

A piece of the code I find on the way:

procedure TContactController.UpdateComponent(Sender: TObject);
var
  I: Integer;
begin
  UpdateComponentActive := True;

  If Sender = pnlBasicInformation
  then begin
    pnlBasicInformation.Caption := UpperCase(Trim(Format('%s %s', [CurrentContact.Name, CurrentContact.FirstName])));
    If CurrentContact.HomeAddress.PostalCode.City.CityDescription[ApplicationManager.CurrentLanguageId] <> ''
    then pnlBasicInformation.Caption := Format('%s - %s',
                                               [pnlBasicInformation.Caption,
                                                UpperCase(CurrentContact.HomeAddress.PostalCode.City.CityDescription[ApplicationManager.CurrentLanguageId])]);
    pnlBasicInformation.Caption := StringReplace(pnlBasicInformation.Caption, '&', '&&', [rfReplaceAll]);
  end;

  If Sender = gbtnLock
  then begin
    If CurrentContact.Locked
    then gbtnLock.ImageIndex := 7
    else gbtnLock.ImageIndex := 19;
  end;

  If Sender = edtInternalReference
  then edtInternalReference.Text := CurrentContact.InternalReference;

  If Sender = edtExternalReference
  then edtExternalReference.Text := CurrentContact.ExternalReference;

  If Sender = edtFirstName
  then edtFirstName.Text := CurrentContact.FirstName;

  If Sender = edtName
  then edtName.Text := CurrentContact.Name;

  If Sender = edtSubName
  then edtSubName.Text := CurrentContact.SubName;

  If Sender = cbContactFunction
  then begin
    cbContactFunction.ItemIndex := -1;
    For I := 0 to cbContactFunction.Items.Count-1 do
    begin
      If TContactFunction(cbContactFunction.Items.Objects[I]).Id = CurrentContact.ContactFunctionId
      then begin
        cbContactFunction.ItemIndex := I;
        break;
      end;
    end;
  end;

  If Sender = cbLanguage
  then begin
    cbLanguage.ItemIndex := -1;
    For I := 0 to cbLanguage.Items.Count-1 do
    begin
      If TLanguage(cbLanguage.Items.Objects[I]).Id = CurrentContact.LanguageId
      then begin
        cbLanguage.ItemIndex := I;
        break;
      end;
    end;
  end;

  If Sender = cbSalutation
  then begin
    cbSalutation.ItemIndex := -1;
    For I := 0 to cbSalutation.Items.Count-1 do
    begin
      If TSalutation(cbSalutation.Items.Objects[I]).Id = CurrentContact.SalutationId
      then begin
        cbSalutation.ItemIndex := I;
        break;
      end;
    end;
  end;

  If Sender = edtCallingCodeMobilePhone
  then edtCallingCodeMobilePhone.Text := CurrentContact.CallingCodeMobilePhone;

  If Sender = edtMobilePhone
  then begin
    edtMobilePhone.Text := CurrentContact.MobilePhone;
    edtMobilePhone.OnKeyPress := OnPhoneNumberKeyPress;
  end;

  If Sender = gbtnMobilePhone
  then gbtnMobilePhone.Enabled := Trim(CurrentContact.MobilePhone) <> '';

  If Sender = gbtnMobilePhoneSms
  then gbtnMobilePhoneSms.Enabled := Trim(CurrentContact.MobilePhone) <> '';

  If Sender = edtBirthDate
  then edtBirthDate.Text := Format('dd/mm/yyyy', [CurrentContact.BirthDate]);

  If Sender = rbGenderM
  then rbGenderM.Checked := (CurrentContact.Gender = 0);

  If Sender = rbGenderV
  then rbGenderV.Checked := (CurrentContact.Gender = 1);

  If Sender = edtIdentityCardNumber
  then edtIdentityCardNumber.Text := CurrentContact.IdentityCardNumber;

  If Sender = edtNationalNumber
  then edtNationalNumber.Text := CurrentContact.NationalNumber;

  If Sender = imgProfilePhoto
  then imgProfilePhoto.Picture.Assign(ProfilePhoto.Picture.Graphic);

  If Sender = gbtnRemovePhoto
  then gbtnRemovePhoto.Enabled := PhotoManager.PhotoExists(pmContact, CurrentContact.Id);

  If Sender = edtRemarks
  then edtRemarks.Text := CurrentContact.Remarks;

  If Sender = edtInfo
  then edtInfo.Text := CurrentContact.Info;

  If Sender = edtRowVersion
  then edtRowVersion.Text := Format('dd/mm/yyyy', [CurrentContact.RowVersion]);

  UpdateComponentActive := False;
end;

therefore I wanted to change the code with if / else as first changes

it is not clear what you are asking, or what advice you are seeking. — Paul Stoner, Sep 12 '17 at 14:07
If you can guarantee that only one of the if statements succeed then the else approach is indeed statistically faster. In the case that Sender is always equal to the element tested last you will gain nothing. — Uwe Raabe, Sep 12 '17 at 14:08
The two versions are semantically different. Still, this smells like premature optimization. — David Heffernan, Sep 12 '17 at 14:09
@UweRaabe: Yes it go only in one statement. That's the reason I've change the code. But I was not sure if this was the right optimization — Ravaut123, Sep 12 '17 at 14:27
Are you aware that the two blocks of code have different meaning? — David Heffernan, Sep 12 '17 at 14:48
@pen All tests are performed in the first block. The tests are performed in the second block until the first one evaluates true. Then no more tests are performed. — David Heffernan, Sep 12 '17 at 15:51
Does the second example run faster when you test it? If so, then there's your answer. If not, then there's your answer. What question do you need a bunch of strangers on the Internet to answer for you? Perhaps you instead intended to ask [how to measure the running time of a block of code](https://stackoverflow.com/q/6030586/33732)? — Rob Kennedy, Sep 12 '17 at 16:21
@RobKennedy: I can use the TStopwatch to measure the time. I must change my question. — Ravaut123, Sep 12 '17 at 19:52
What does the `// to something` do specifically? There may be a better way to accomplish what you're trying to do, but you've not explained to us what you're trying to do. I *can* say I've never once written a bunch of code that said `if Sender = this then else if Sender = that then else...`, because there's almost always a better way to do things. I think you may be trying to solve an XY problem, and that if you fix X you won't need this Y solution. — Ken White, Sep 12 '17 at 20:11
@Ken; mostly change the enable or visible of the component or add some value to the component. — Ravaut123, Sep 12 '17 at 20:22
Then my suggestion is that you [edit] your question and make it clear what you're asking. Changing enabled or visible can be done with two or three lines of code that will work on every single control that has an enabled or visible property. Not two or three for each control, but two or three for *all* controls. When you edit, mention what specific version of Delphi you're using (especially if it isn't the current one), as functionality and features in RTTI differ. — Ken White, Sep 12 '17 at 20:23
Performance is irrelevant here. You are completely missing the big picture. The real problem is the code somewhere else that calls ProcessMessages and makes your update uber method re-entrant. Fix your design and this messy method will vanish. But stop worrying about a non existant performance problem and address the real problem. — David Heffernan, Sep 14 '17 at 06:47

score 4 · Answer 1 · answered Sep 12 '17 at 21:16

Your proposal is correct and can save you a few CPU instruction executions (if you care that much about performance). But only if all the expressions are for equality. It's because the first code block always evaluates all the expressions (no matter if one evaluates to true) whilst your proposed code does not.

But in this case I would review the code and try to find as much as the controls you work with have in common and group them accordingly (if possible).

score 2 · Answer 2 · answered Sep 13 '17 at 07:05

A possible alternative would be to set the Tag property of the different edit fields to distinct numbers, and use a case (Sender as TEdit).Tag of-clause. This internally will use a jump-table and avoid having to work through a series of if-statements.

Another possibility, if you feel up for is, is the hard way and implement your own class inheriting from TEdit (or TCustomEdit) and add an extra event-handle just for this exact case where you now call UpdateComp.

Faster code If/then

2 Answers2