Code replication

[Vital]

I have just a comment about some code replication that I have found over many unit. I know this may not be a priority but I thing it worth to be considered here.

I have found, as an exemple that over many unit the same funtion are replicated many time. They do exactly the same.

Min() and Max() for exemple apear at leat 8 time. Here is the code where I found them.



Unit components\codetools\basiccodetools.pas

function Min(i1, i2: integer): integer;
begin
  if i1<i2 then Result:=i1 else Result:=i2;
end;

function Max(i1, i2: integer): integer;
begin
  if i1>i2 then Result:=i1 else Result:=i2;
end;



Unit components\rx\mrulist.pas

Function Max(A,B : Integer) : Integer;

begin
  If A>B then
    Result:=A
  else
    Result:=B;
end;

Function Min(A,B : Integer) : Integer;

begin
  If A>B then
    Result:=B
  else
    Result:=A;
end;


components\synedit\syncompletion

procedure TSynBaseCompletionForm.Paint;
var
  i: integer;

  function Min(a, b: integer): integer;
  begin
    if a < b then
      Result := a
    else
      Result := b;
  end;


components\synedit\syneditmiscprocs

function Max(x, y: integer): integer;
begin
  if x > y then Result := x else Result := y;
end;

function Min(x, y: integer): integer;
begin
  if x < y then Result := x else Result := y;
end;

function MinMax(x, mi, ma: integer): integer;
begin
  if (x < mi) then Result := mi
    else if (x > ma) then Result := ma else Result := x;
end;



Syntextdrawer

function Min(x, y: integer): integer;
begin
  if x < y then Result := x else Result := y;
end;


function CompareMem(P1, P2: Pointer; Length: Integer): Boolean; assembler;


Unit components\turbopower_ipro\iphtml.pas

function MaxI2(const I1, I2: Integer) : Integer;
begin
  Result := I1;
  if I2 > I1 then
    Result := I2;
end;

function MaxI3(const I1, I2, I3: Integer) : Integer;
begin
  if I2 > I1 then
    if I3 > I2 then
      Result := I3
    else
      Result := I2
  else
    if I3 > I1 then
      Result := I3
    else
      Result := I1;
end;

function MinI2(const I1, I2: Integer) : Integer;
begin
  Result := I1;
  if I2 < I1 then
    Result := I2;
end;



Unit components\turbopower_ipro\ipstrms.pas

function MinLong(A, B : Longint) : Longint;
begin
  if A < B then
    Result := A
  else
    Result := B;
end;


Unit fpcsrc\packages\extra\gtk2\exemples\gtk_demo\gtk_demo.pas

  function min (d1, d2: double): double;
  begin
    if d1 > d2 then  min := d2
    else min := d1;
  end;

  function max (d1, d2: double): double;
  begin
    if d1 < d2 then max := d2
    else max := d1;
  end;


Unit fpcsrc\packages\extra\ncurses\edit_demo.pp

Function Min(i1,i2 : integer) : integer;
Begin
  If i1 < i2 Then
     Min := i1
  Else
     Min := i2;
End;


Unit fpcsrc\packages\extra\numlib\spl.pas

function Min(a, b:ArbInt): ArbInt;
begin if a<b then Min := a else Min := b end;


Unit fpcsrc\rtl\inc\graph\fills.inc

function max(a, b : Longint) : Longint;
begin
  max := b;
  if (a > b) then max := a;
end;

function min(a, b : Longint) : Longint;
begin
  min := b;
  if (a < b) then min := a;
end;


Unit fpcsrc\rtl\netware\nvserv.pp
function min(para1:longint; para2:longint):longint;cdecl;external 'clib' name 'min';


Unit fpcsrc\rtl\netwlibc\libc.pp

function max(a:longint; b:longint):longint;cdecl;external libc_nlm name 'max';
function min(a:longint; b:longint):longint;cdecl;external libc_nlm name 'min';




Unit fpcsrc\rtl\objpas\math.pp

function MinIntValue(const Data: array of Integer): Integer;
function MaxIntValue(const Data: array of Integer): Integer;

{ Extra, not present in Delphi, but used frequently  }
function Min(a, b: Integer): Integer;
function Max(a, b: Integer): Integer;
function Min(a, b: Cardinal): Cardinal;
function Max(a, b: Cardinal): Cardinal;
function Min(a, b: Int64): Int64;
function Max(a, b: Int64): Int64;
{$ifdef FPC_HAS_TYPE_SINGLE}
function Min(a, b: Single): Single;
function Max(a, b: Single): Single;
{$endif FPC_HAS_TYPE_SINGLE}
{$ifdef FPC_HAS_TYPE_DOUBLE}
function Min(a, b: Double): Double;
function Max(a, b: Double): Double;
{$endif FPC_HAS_TYPE_DOUBLE}
{$ifdef FPC_HAS_TYPE_EXTENDED}
function Min(a, b: Extended): Extended;
function Max(a, b: Extended): Extended;
{$endif FPC_HAS_TYPE_EXTENDED}

function InRange(const AValue, AMin, AMax: Integer): Boolean;
function InRange(const AValue, AMin, AMax: Int64): Boolean;
{$ifdef FPC_HAS_TYPE_DOUBLE}
function InRange(const AValue, AMin, AMax: Double): Boolean;
{$endif FPC_HAS_TYPE_DOUBLE}

function EnsureRange(const AValue, AMin, AMax: Integer): Integer;
function EnsureRange(const AValue, AMin, AMax: Int64): Int64;
{$ifdef FPC_HAS_TYPE_DOUBLE}
function EnsureRange(const AValue, AMin, AMax: Double): Double;


Unit fpcsrc\unix\crt.pp

Function Min(l1,l2:longint):longint;
{
  Return the minimum of l1 and l2
}
begin
  if l1<l2 then
   Min:=l1
  else
   Min:=l2;
end;


Unit fpcsrc\rtl\win32\wininc\base.inc

  function max(a,b : longint) : longint;
    { return type might be wrong }
    var
       if_local1 : longint;
    (* result types are not known *)
    begin
       if a > b then
         if_local1:=a
       else
         if_local1:=b;
       max:=if_local1;
    end;

  { was #define dname(params) def_expr }
  { argument types are unknown }
  { return type might be wrong }
  function min(a,b : longint) : longint;
    { return type might be wrong }
    var
       if_local1 : longint;
    (* result types are not known *)
    begin
       if a < b then
         if_local1:=a
       else
         if_local1:=b;
       min:=if_local1;
    end;



Unit lcl\grid.pas

function Min(const I,J: Integer): Integer;
begin
  if I<J then Result:=I

  else        Result:=J;
end;
function Max(const I,J: Integer): Integer;
begin
  if I>J then Result:=I

  else        Result:=J;
end;


Unit lcl\include\intfbaselcl.inc
Function TWidgetSet.InvalidateFrame(aHandle : HWND; ARect : pRect;
  bErase : Boolean; BorderWidth: integer) : Boolean;

  function Min(i1, i2: integer): integer;
  begin
    if i1<=i2 then Result:=i1 else Result:=i2;
  end;

  function Max(i1, i2: integer): integer;
  begin
    if i1<=i2 then Result:=i2 else Result:=i1;
  end;


I thing it may help to have clean code to just use one of them and have a main unit for this king of function.

I hope this help for quality.

Vital

[Marc]

The reason for this is probably historical.
I don't know it it is still the case, but in the past it was not done to include the math unit if you didn't realy need it since it was large and it required a FPU.
So for the simple functions like Min and Max you wrote a local version. That is what you see.

[Vital]

Thanks you Marc.

Do you think it could be a better practice to use a library where possible instead of local function? I belive it's better for cleaner code, and easier to maintain, but it may have some other raison that I don't know?

[Marc]

Don't know. If you move them to a unit, you have them in 2 different unist and that is also not what you want.

I think for projects like lazarus it is no problem to use the Math unit. For smaller projects you have to try yourself if it has influence on the size of your app.

[mikiwoz]

Well, doesn't FPC crop-out all the unnecessary code for you? E.g. If you include the Math unit in your uses clause, won't fpc include in the binary only the functions you actually use? Of course only when you use smartlinking.
At least that's what I think, correct me if I'm wrong.

[Vital]

Yes, I agree, that should be the way imho. It also be the responsability of the library maintener because I guest some of the code is maintened bye third party developper's group. I don't down how FPC handle functions of a unit so I can't tell but I must agree that as long as smark linking is invoked that should be the case.

May be FPC developpers's should define some rule for developper to follow to have better coding practice. I agree that there's so many function in labrary that is't difficult to be aware of all the stuff. The rule is simple for me : "If a function is already there, somewhere in library you don't have to make it again".

This may be a great responsability for a programmer because some time you may not found a function name as expected. I. E. You may not found a function even this function is aready there just because you do not know where to find it or how to spell it.


Vital