snip<=-
snip<=-
everything is correct, but I'm getting a 'File not open' error when it's run.
everything is correct, but I'm getting a 'File not open' error when it run.
Do you assign the path to the trivia.txt file anywhere? ie: I have a routine called exedir which sends dos the directory the program is run from.
Hi All,
I'm having an issue with some code that I'm writing. It looks like everything is correct, but I'm getting a 'File not open' error when it's run.
Here is a snippet of the code:
snip<=-
assignfile(trivia,'trivia.txt');
try
{$I-}
reset(trivia);
{$I+}
if IoResult<>0 then
begin
Writeln('Unable to open file ');
halt;
end;
while not eof(trivia) do
begin
readln(trivia,linetmp1);
if AnsiStartsStr(IntToStr(qnum),linetmp1) then
begin
repeat
readln(trivia,linetmp1);
until AnsiStartsStr(y,linetmp1);
w:=ExtractDelimited(2,linetmp1,[' ']);
if upcase(x)=upcase(w) then corans:=true;
end;
close(trivia);
end;
except
on E: EInOutError do
writeln('File handling error occurred. Details: ', E.Message);
end;
try
{$I-}
reset(trivia);
{$I+}
Why do {$I-}? Since you're already in a `try` block, just let it
throw an exception if there's an error.
while not eof(trivia) do
Hmm. What if you hit EOF somewhere in here? I don't think that's
your problem, but it's a bug.
end;
close(trivia);
So you close the file and then loop again? Surely the file won't
be open on the next iteration of the loop. I suspect this is your
problem. That is, you actually read from the file but then close
it and try to check it for EOF, at which point it's closed: hence
your error.
end;
I think you want to close *after* this `end;`.
except
on E: EInOutError do
writeln('File handling error occurred. Details: ', E.Message);
end;
This is fine. Just let it catch your errors, no? From your follow-on messages, that's what was happening anyway?
At any rate, don't close your file while you're still reading it.
qstr := IntToStr(qnum);
upperx := upcase(x);
Note that you continually re-evaluate these throughout your loop. Surely the compiler is smart enough to figure that out and optimize it away for you, but there's no reason not to simply cache the results once, and I argue it reads a bit better to give those expressions names.
while not EOF(trivia) and corans = false do
begin
It appears that the purpose of this loop is to set 'corans' to true
if you find some text in a file, but you never bother to set it to
false again. This was just a snippet, but one may reasonably assume
that 'corans' was FALSE when the loop starts and that if it ever becomes true, we may short-circuit loop termination, since setting it to TRUE
is the most lasting side-effect of the loop.
readln(trivia, linetmp1);
if not AnsiStartsStr(qstr, linetmp1) then
continue;
This is what's called a 'guard clause': check for the simple stuff up front and only continue on if the interesting condition is met. Your
loop doesn't do much unless `AnsiStartsStr(qstr, linetmp1)` is true; so just skip to the next iteration if it's not.
while not EOF(trivia) do
begin
readln(trivia, linetmp1);
if AnsiStartsStr(qstr, linetmp1) then
break;
end;
Note that in this second loop, we check for EOF(trivia) and just
terminate the loop if AnsiStartsStr() is true. The loop *appears*
to skip lines for which that condition doesn't hold.
snip<=-
snip<=-
Note that we check again for EOF: the loop above may have terminated
for either of two reasons: we reached EOF, or we found another `AnsiStartsStr` line that's true. If we hit EOF, then we know that
the second condition *is not true* so we exit the outer loop.
w := ExtractDelimited(2, linetmp1, [' ']);
if upcase(w) = upperx then
corans := true;
This is the most interesting bit of logic, but what happens if `ExtractDelimited` fails to find a second space-delimited field?
You should add some error checking here for poorly formed input.
end;
close(trivia);
Note that we close trivia *after* we terminate the outer loop.
on E: EInOutError do
writeln('Input/Output error: ', E.Message);
end;
Note that this single `except` block handles all IO errors from the loop(s), including failure to open `trivia.txt`. Note also that the message is a bit more terse, but still understandable.
readln(trivia, linetmp1);
if not AnsiStartsStr(qstr, linetmp1) then
continue;
This is what's called a 'guard clause': check for the simple stuff up front and only continue on if the interesting condition is met. Your loop doesn't do much unless `AnsiStartsStr(qstr, linetmp1)` is true; just skip to the next iteration if it's not.
Good idea.
while not EOF(trivia) do
begin
readln(trivia, linetmp1);
if AnsiStartsStr(qstr, linetmp1) then
break;
end;
Note that in this second loop, we check for EOF(trivia) and just terminate the loop if AnsiStartsStr() is true. The loop *appears*
to skip lines for which that condition doesn't hold.
Yes, what it's doing, is looking for a specific trivia question number, and then looking for the correct answer within that file. This second
loop is actually looking for the first line, following the correct question number, that starts with an '*'. That line contains the correct answer to the question. The file is in the format of:
w := ExtractDelimited(2, linetmp1, [' ']);
if upcase(w) = upperx then
corans := true;
This is the most interesting bit of logic, but what happens if `ExtractDelimited` fails to find a second space-delimited field?
You should add some error checking here for poorly formed input.
I know because I created the file. :) Seriously though. You're right, there should be some error checking in place.
Thank you so much. I really appreciate all of the assistance.
At any rate, don't close your file while you're still reading it.
On 01/27/18, tenser said the following...
At any rate, don't close your file while you're still reading it.
Sort of like a book? ;)
Sysop: | altere |
---|---|
Location: | Houston, TX |
Users: | 57 |
Nodes: | 4 (0 / 4) |
Uptime: | 03:47:27 |
Calls: | 513 |
Files: | 6,920 |
Messages: | 285,716 |