The proper way

QC20N
QC20N used Ask the Experts™
on
I have this code that I hope some can help me to write proper way. The code does what it should do, but I'm not sure if this is the proper way. Hopefully some of it could be rewrite to a function.

Thank you
procedure TForm1.ADInfoUpdate;
var User : IADsuser; LineList : TStringList; sADPath, sdesc : string; Query : TADOQuery; VarArray, Value : Variant; i, HighBound : integer; Updated: boolean;
begin
  Updated := false;
  ADOQuery1.SQL.Clear;
  ADOQuery1.SQL.Add('SELECT REASON, ADPATH FROM USERINACTIVE WHERE ADUPDATE = :ADUpdate');
  ADOQuery1.Parameters.ParamByName('ADUpdate').Value := False;
  ADOQuery1.Open;
  try
    while not ADOQuery1.Eof do
    begin
      try
        if SUCCEEDED(ADsGetObject(ADOQuery1.FieldByName('ADPATH').AsString, IADsUser, User)) then
        begin
          LineList := TStringList.Create;
          LineList.Delimiter := ',';
          LineList.CaseSensitive := True;
          try
            sdesc := User.Description;
            try
              LineList.DelimitedText := Uppercase(StringReplace(sDesc,' ','_',[rfReplaceAll]));
              for I := 0 to LineList.Count - 1 do
              begin
                if pos('BOSS',LineList[i]) <> 0 then
                begin
                  LineList[i] := StringReplace('BOSS: ' + ADOQuery1.FieldByName('REASON').AsString,' ','_',[rfReplaceAll]);
                  Updated := true;
                end;
              end;
              for I := 0 to LineList.Count - 1 do
              begin
                if pos('_',LineList[i]) = 0 then
                  LineList[i] := QuotedStr(LineList[i])
                else
                  LineList[i] := StringReplace(LineList[i],'_',' ',[rfReplaceAll]);
              end;
              User.Description := Linelist.DelimitedText;
              User.SetInfo;
            finally
              User := nil;
              LineList.Free;
            end;
          except
            user.Description := '"BOSS: ' + ADOQuery1.FieldByName('REASON').AsString+'"';
            User.SetInfo;
          end;
        end;
      except
        on EOleException do
        begin
          // ShowMessage('Test');
        end;
      end;
      ADOQuery1.Next;
    end;
  finally
    ADOQuery1.Close;
  end;
end;

Open in new window

Comment
Watch Question

Do more with

Expert Office
EXPERT OFFICE® is a registered trademark of EXPERTS EXCHANGE®

Commented:
The code just viewed fine,
except if you want to generalized the procedure or function
procedure TForm1.ADInfoUpdate(var User : IADsuser; sADPath, sdesc : string );
var LineList : TStringList;  Query : TADOQuery; VarArray, Value : Variant; i, HighBound : integer; Updated: boolean;
...
...
Software Engineer
Commented:
You dont need to do the loop twice.
Take a look at this
procedure TForm1.ADInfoUpdate;
var
  User : IADsuser;
  LineList : TStringList;
  sADPath, sdesc : string;
  Query : TADOQuery;
  VarArray, Value : Variant;
  i, HighBound : integer;
  Updated: boolean;
begin
  Updated := false;
  ADOQuery1.Active := False;
  ADOQuery1.SQL.Clear;
  ADOQuery1.SQL.Add('SELECT REASON, ADPATH FROM USERINACTIVE WHERE ADUPDATE = :ADUpdate');
  ADOQuery1.Parameters.ParamByName('ADUpdate').Value := False;
  ADOQuery1..Active := True;
  try
    while not ADOQuery1.Eof do
    begin
      try
        if SUCCEEDED(ADsGetObject(ADOQuery1.FieldByName('ADPATH').AsString, IADsUser, User)) then
        begin
          LineList := TStringList.Create;
          try
            LineList.Delimiter := ',';
            LineList.CaseSensitive := True;

            try
              sdesc := User.Description;
              try
                LineList.DelimitedText := Uppercase(StringReplace(sDesc,' ','_',[rfReplaceAll]));
                for I := 0 to LineList.Count - 1 do
                begin
                  if pos('BOSS',LineList[i]) <> 0 then
                  begin
                    LineList[i] := StringReplace('BOSS: ' + ADOQuery1.FieldByName('REASON').AsString,' ','_',[rfReplaceAll]);
                    Updated := true;
                  end;

                  if pos('_',LineList[i]) = 0 then
                    LineList[i] := QuotedStr(LineList[i])
                  else
                    LineList[i] := StringReplace(LineList[i],'_',' ',[rfReplaceAll]);
                end;

                User.Description := Linelist.DelimitedText;
                User.SetInfo;
              finally
                User := nil;
              end;
            except
              user.Description := '"BOSS: ' + ADOQuery1.FieldByName('REASON').AsString+'"';
              User.SetInfo;
            end;
          finally
            FreeAndNil(LineList);
          end;
        end;
      except
        on EOleException do
        begin
          // ShowMessage('Test');
        end;
      end;
      ADOQuery1.Next;
    end;
  finally
    ADOQuery1.Active := False;
  end;
end;

Open in new window

Author

Commented:
@ewangoya:

I see your point. Nice.

I know I sad my code runs as it should be, but when I run my code agan I see a small problem.

My startpoint in the description is:
"VACATION","BOSS: VACATION","TEST"

but the endresult is:
'VACATION',"BOSS: VACATION LEAVE",'TEST'

I can't figure out why every word isen't enclosed in quotation marks.

I hope this isen't to much off topic
Ephraim WangoyaSoftware Engineer

Commented:

Double quotes is only used when you have spaces in your string

Do more with

Expert Office
Submit tech questions to Ask the Experts™ at any time to receive solutions, advice, and new ideas from leading industry professionals.

Start 7-Day Free Trial