What is the optimal way to check if some values must be replaced or not?-CodePudding

I am updating some product prices in a ERP program in Delphi and I would like to check if new prices with smaller or zero values should be replaced. I have written it in two ways (the first one is the existing code and the second one is an alternate way to implement it). Code examples are:

1st example:

if DlgBuff.AsInteger[0, FldReplSmall]=0 then             // Replace Smaller Values: No
begin
  if OldPriceW>PriceW then
    PriceW:=OldPriceW;
  if OldPriceR>PriceR then
    PriceR:=OldPriceR;
end
else if DlgBuff.AsInteger[0, FldReplSmall]=2 then        // Replace smaller non zero values
begin
  if (OldPriceW>PriceW) and (PriceW=0) then
    PriceW:=OldPriceW;
  if (OldPriceR>PriceR) and (PriceR=0) then
    PriceR:=OldPriceR;
end;

2nd example:

if OldPriceW>PriceW then
  if (DlgBuff.AsInteger[0, FldReplSmall]=0) or ((DlgBuff.AsInteger[0, FldReplSmall]=2) and (PriceW=0)) then
    PriceW:=OldPriceW;
if OldPriceR>PriceR then
  if (DlgBuff.AsInteger[0, FldReplSmall]=0) or ((DlgBuff.AsInteger[0, FldReplSmall]=2) and (PriceR=0)) then
  PriceR:=OldPriceR;

What is, in your opinion, the correct way to write something like this? Correct meaning the more efficient way.

The first example is the pre-existing code. I added the else block to handle the replacement of smaller non zero values (Replace Smaller Values: No was already there).

Then I also though about writing it in a way that is more compact but harder to understand at first glance (maybe?).. What do you think is the more appropriate way to write something like this?

I think the second way is better because it has less IF checks and less code that is repeated, but the first one seems to be more clear and understandable for somebody.

CodePudding user response：

Instead of comparing which number is higher by yourself using bunch of if statements use Max function from System.Math unit.

So code for keeping higher prices would look like this

if DlgBuff.AsInteger[0, FldReplSmall]=0 then             // Replace Smaller Values: No
begin
  PriceW := Max(OldPriceW, PriceW);
  PriceR := Max(OldPriceR, PriceR);
end;

As for replacing only zero valued prices. Why are you even checking old price is larger than the new one? If the new price is zero than the only price can either be same (old price was also zero) or larger. In both cases you can simply assign old price to the new one.

CodePudding user response：

case DlgBuff.AsInteger[0, FldReplSmall] of
  0: begin
       PriceW:=Greatest(PriceW,OldPriceW);
       PriceR:=Greatest(PriceR,OldPriceR);
     end;
  2: begin
       if (PriceW=0) than PriceW:=Greatest(PriceW,OldPriceW);
       if (PriceR=0) than PriceR:=Greatest(PriceR,OldPriceR);
     end;
  end;