Recent

Author Topic: [SOLVED] Improvement of procedure SysGetEnvironmentList  (Read 658 times)

lagprogramming

  • Sr. Member
  • ****
  • Posts: 403
[SOLVED] Improvement of procedure SysGetEnvironmentList
« on: July 14, 2023, 08:29:18 am »
packages/fcl-base/src/custapp.pp has the following procedure:
Code: Pascal  [Select][+][-]
  1. Procedure SysGetEnvironmentList(List : TStrings;NamesOnly : Boolean);
  2.  
  3. var
  4.    s : string;
  5.    i,l,j,count : longint;
  6.  
  7. begin
  8.   count:=GetEnvironmentVariableCount;
  9.   if count>0 then
  10.     for j:=1 to count  do
  11.      begin
  12.        s:=GetEnvironmentString(j);
  13.        l:=Length(s);
  14.        If NamesOnly then
  15.           begin
  16.             I:=pos('=',s);
  17.             If (I>0) then
  18.               S:=Copy(S,1,I-1);
  19.           end;
  20.        List.Add(S);
  21.     end;
  22. end;
Variable l is declared and a value is assigned to it but it's never used.
Also, the line if count>0 then adds an unnecessary conditional jump. Either it remains and the for loop is replaced with a repeat loop, either the if statement is removed.

1/2 The following patch removes the l variable declaration, the l:=Length(s); line and the if count>0 then line.
Code: Pascal  [Select][+][-]
  1. diff --git a/packages/fcl-base/src/custapp.pp b/packages/fcl-base/src/custapp.pp
  2. index adb0ff46ae..c6a4b064b8 100644
  3. --- a/packages/fcl-base/src/custapp.pp
  4. +++ b/packages/fcl-base/src/custapp.pp
  5. @@ -176,22 +176,20 @@ function TCustomApplication.GetExeName: string;
  6.  
  7.  var
  8.     s : string;
  9. -   i,l,j,count : longint;
  10. +   i,j,count : longint;
  11.  
  12.  begin
  13.    count:=GetEnvironmentVariableCount;
  14. -  if count>0 then
  15. -    for j:=1 to count  do
  16. -     begin
  17. -       s:=GetEnvironmentString(j);
  18. -       l:=Length(s);
  19. -       If NamesOnly then
  20. -          begin
  21. -            I:=pos('=',s);
  22. -            If (I>0) then
  23. -              S:=Copy(S,1,I-1);
  24. -          end;
  25. -       List.Add(S);
  26. +  for j:=1 to count  do
  27. +    begin
  28. +    s:=GetEnvironmentString(j);
  29. +    If NamesOnly then
  30. +      begin
  31. +      I:=pos('=',s);
  32. +      If (I>0) then
  33. +        S:=Copy(S,1,I-1);
  34. +      end;
  35. +    List.Add(S);
  36.      end;
  37.  end;

2/2 One last thing. Unlike the SysGetEnvironmentList functions in packages/fcl-base/src/go32v2/custapp.inc and packages/fcl-base/src/os2/custapp.inc which contain the code
If (I>1) then S:=Copy(S,1,I-1);
the SysGetEnvironmentList function in fcl-base/src/custapp.pp contains the code
If (I>0) then S:=Copy(S,1,I-1);
I was thinking that maybe they should look alike, I mean compare I with the same value.
« Last Edit: July 14, 2023, 12:59:10 pm by lagprogramming »


Josh

  • Hero Member
  • *****
  • Posts: 1254
Re: Improvement of procedure SysGetEnvironmentList
« Reply #2 on: July 14, 2023, 10:59:58 am »
no knowing how the diff works, but curious as why the mod has line
Quote
@@ -176,22 +176,20 @@ function TCustomApplication.GetExeName: string;
does it means it for function GetExeName rather than procedure SysGetEnvironmentList ?

I agree keep the for loop rather than a repeat or while loop, if memory serves its faster and can optimize better.

you could remove the count var too
Code: Pascal  [Select][+][-]
  1. Procedure SysGetEnvironmentList(List : TStrings;NamesOnly : Boolean);
  2. var
  3.   s : string;
  4.   i,j : longint;
  5. begin
  6.   for j:=1 to GetEnvironmentVariableCount  do
  7.   Begin
  8.     s:=GetEnvironmentString(j);
  9.     If NamesOnly then
  10.     begin
  11.       i:=pos('=',s);
  12.       If (i>0) then S:=Copy(S,1,i-1);
  13.     end;
  14.     List.Add(S);
  15.   end;
  16. end;

The best way to get accurate information on the forum is to post something wrong and wait for corrections.

lagprogramming

  • Sr. Member
  • ****
  • Posts: 403
Re: Improvement of procedure SysGetEnvironmentList
« Reply #3 on: July 14, 2023, 12:58:54 pm »
no knowing how the diff works, but curious as why the mod has line
Quote
@@ -176,22 +176,20 @@ function TCustomApplication.GetExeName: string;
does it means it for function GetExeName rather than procedure SysGetEnvironmentList ?
Indeed, reading that text line adds confusion or misleading. It's not what we might expect from it.

 

TinyPortal © 2005-2018