CODEWITHSUNDEEP
delphi
Aidan Quinn
Aidan Quinn

Reputation: 1188

Searching for an element in an array has no results?

Firstly, I know I'm going to get shouted at for using obsolete Delphi methods, but It was the way I was taught and I'm just practicing these things for an exam coming up.

Anyway, I'm reading a bunch of names via a textfile into an array. The user then has the option to search for a name in the text file, and it will either return that the name is there, or isn't.

Here's the code:

procedure TForm1.btnSearchClick(Sender: TObject);
var
  myFile : TextFile;
  Search : string;
  k : Integer;
  arrNames : array [1..200] of string;
  bFound : Boolean;
begin
  AssignFile (myFile, 'names.txt');

  reset(myFile);

  Search := UpperCase(InputBox('Search a Name', '', 'Tiffani Bressler'));

  k := 1;

  bFound := False;

  while not Eof(myFile) or bFound = False do begin

      Readln(myFile, arrNames[k]);

      if Search = UpperCase(arrNames[k]) then

      begin

        bFound := true;

      end;

      lblLineSearched.Caption := 'Line searched: ' + IntToStr(k);

      inc(k);

    end;

    if bFound = True then

      ShowMessage('You searched for: ' + Search + ' and it was found on line: ' + IntToStr(k))

      else ShowMessage('You searched for: ' + Search + ' and it was not found');

  CloseFile(myFile);

end;

The problem is as soon as I press the search button (i directly copy a name from the text file itself), i get the message saying Name not found.

What am I doing wrong?

edit: Where I put the 'LabelSearched', that doesn't even change after i have searched.

Upvotes: 0

Views: 172

Answers (2)

David Heffernan
David Heffernan

Reputation: 612964

The logical expression is wrong. The equality test is binding to the whole expression. It's the same as

((not Eof(myFile)) or bFound) = False

Learn about this by studying the table of operator precedence. Note that = has lower precedence than the other operators in this expression. And that not has higher preference than or which is why I added the parens around not Eof().

Since Eof returns False this test always fails and the body of the while loop is never entered. You mean

not Eof(myFile) or (bFound = False)

But it is idiomatically poor to test booleans against true or false. So I'd do this like so:

not Eof(myFile) or not bFound

Even then that is still wrong. You need both conditions to be true to enter a new iteration. It's no good entering the loop body if Eof returns true. So you need this condition:

not Eof(myFile) and not bFound

Personally I think I'd try to do without the bFound local and use break to escape from the loop, but that is perhaps more a matter of personal taste.

If you want to move on from legacy Pascal I/O then you might contemplate TStreamReader. That will allow you to keep the same while loop reading one line at a time.

I'm sure I've said this before, quite possibly to you, but your array is not only pointless and wasteful, but dangerous. You consider one line at a time. Use the same string local variable to read each line. As it stands your code is bait for a buffer overrun error.

Ah yes, here's the question: Access of violation at address 00404094 with AssignFile(); Please review the answer you accepted to that question.

Upvotes: 6

Peter Kostov
Peter Kostov

Reputation: 951

It's not a direct answer to your question, but you can use another approach - a TStringList.

Something like:

procedure Search(const aFileName: String; const aSearchString: String);
var
  index: Integer;
  sl: TStringList;    
begin
  sl := TStringList.Create;
  try        
    sl.LoadFromFile(aFileName);

    // case sensitive search
    sl.CaseSensitive := True;

    index := sl.IndexOf(aSearchString);

    if index > -1 then
      ShowMessage(Format('String found at line %d', [index + 1])) else
      ShowMessage('String not found!');

  finally
    sl.Free;
  end;
end;

Upvotes: 4

Related Questions