• Pascal Headache...

    From Black Panther@21:1/186 to All on Sat Jan 27 12:16:49 2018
    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;

    snip<=-

    I'm assigning the file with 'assignfile(trivia,'trivia.txt');
    I'm using 'reset(trivia);
    I even have it contained in a 'try ... except' to capture any errors
    Before adding the 'try ... except', I just had the {$I-}{$i+}, and that
    wasn't giving any errors on it's own.

    The program gets to the line 'while not eof(trivia) do' and errors with the 'File not open' error.

    I know I'm missing something really simple here, but it's been eluding me for the last three days...

    Any help would be greatly appreciated.

    Thanks,


    ---

    Black Panther
    a.k.a. Dan Richter
    Sysop - Castle Rock BBS (RCS)
    The sparrows are flying again....

    --- Mystic BBS v1.12 A38 2018/01/01 (Linux/64)
    * Origin: Castle Rock BBS - bbs.castlerockbbs.com (21:1/186)
  • From Tiny@21:1/130.4 to Black Panther on Sat Jan 27 17:13:20 2018
    everything is correct, but I'm getting a 'File not open' error when it's 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.

    Var
    Trivia := Text;
    Begin
    Assign(Trivia, exedir + 'trivia.txt');
    Reset(Trivia)
    etc

    Or try assign(Trivia, 'c:\full\path\to\trivia.txt');

    Shawn


    --- MagickaBBS v0.9alpha (Linux/armv7l)
    * Origin: A Tiny slice o pi (21:1/130.4)
  • From Black Panther@21:1/186 to Tiny on Sat Jan 27 15:29:19 2018
    On 01/27/18, Tiny said the following...

    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.

    The trivia.txt file is located in the working directory where the program is located. I did try adding the path, but got the same result. I even added:

    runpath:=GetCurrentDir; //earlier in the code

    ChDir(runpath); //right before assigning and resetting the file...

    I even went as far as adding:

    if FileExists('trivia.txt') then bool:=true;
    if bool=true then writeln('The file exists');

    The program is able to find the file, but for the life of me, I can't figure out why it not opening it...

    What's really strange, is I open and work with that file in other parts of
    the same program, with no problems...


    ---

    Black Panther
    a.k.a. Dan Richter
    Sysop - Castle Rock BBS (RCS)
    The sparrows are flying again....

    --- Mystic BBS v1.12 A38 2018/01/01 (Linux/64)
    * Origin: Castle Rock BBS - bbs.castlerockbbs.com (21:1/186)
  • From Black Panther@21:1/186 to All on Sat Jan 27 15:33:35 2018
    Hi All,

    Just to add to this, why is it that opening the file is erroring out with an EInOutError, but not having any problems whith IoResult?

    Something strange, is afoot...


    ---

    Black Panther
    a.k.a. Dan Richter
    Sysop - Castle Rock BBS (RCS)
    The sparrows are flying again....

    --- Mystic BBS v1.12 A38 2018/01/01 (Linux/64)
    * Origin: Castle Rock BBS - bbs.castlerockbbs.com (21:1/186)
  • From tenser@21:1/112 to Black Panther on Sat Jan 27 22:12:00 2018
    On 01/27/18, Black Panther said the following...

    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;

    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
    begin
    readln(trivia,linetmp1);
    if AnsiStartsStr(IntToStr(qnum),linetmp1) then
    begin
    repeat
    readln(trivia,linetmp1);
    until AnsiStartsStr(y,linetmp1);

    Hmm. What if you hit EOF somewhere in here? I don't think that's
    your problem, but it's a bug.

    w:=ExtractDelimited(2,linetmp1,[' ']);
    if upcase(x)=upcase(w) then corans:=true;
    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.

    While you're at it, your loop is kind of fragile and probably needs
    a rewrite. More on that in a minute.

    --- Mystic BBS v1.12 A38 2018/01/01 (Windows/32)
    * Origin: ACiD Telnet HQ / blackflag.acid.org (21:1/112)
  • From tenser@21:1/112 to tenser on Sat Jan 27 22:33:44 2018
    Here's an example rewrite of your loop. Note I haven't tried to compile
    it, so there may be silly syntax errors. But I think the logic is more
    robust now, though still imperfect.

    qstr := IntToStr(qnum);
    upperx := upcase(x);

    assignFile(trivia, 'trivia.txt');

    try
    reset(trivia);
    while not EOF(trivia) and corans = false do
    begin
    readln(trivia, linetmp1);
    if not AnsiStartsStr(qstr, linetmp1) then
    continue;
    while not EOF(trivia) do
    begin
    readln(trivia, linetmp1);
    if AnsiStartsStr(qstr, linetmp1) then
    break;
    end;
    if EOF(trivia) then
    break;
    w := ExtractDelimited(2, linetmp1, [' ']);
    if upcase(w) = upperx then
    corans := true;
    end;
    close(trivia);
    end;
    except
    on E: EInOutError do
    writeln('Input/Output error: ', E.Message);
    end;

    Here it is again with some notes....

    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.

    assignFile(trivia, 'trivia.txt');

    try
    reset(trivia);

    Note: just let the exception handler below deal with this.

    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.

    if EOF(trivia) then
    break;

    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.

    end;
    except
    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.

    --- Mystic BBS v1.12 A38 2018/01/01 (Windows/32)
    * Origin: ACiD Telnet HQ / blackflag.acid.org (21:1/112)
  • From Black Panther@21:1/186 to tenser on Sat Jan 27 20:49:27 2018
    On 01/27/18, tenser said the following...

    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.

    The {$I-} was added when I started having issues with the 'file not open'. I normally just use the 'try' block.

    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.

    That's a good point. There should be another check in there.

    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.

    Oh my ***!... For whatever reason, I was looking at why the file wasn't being opened at the beginning. I wasn't even thinking about the file being closed within the loop... Ok, I feel like an idiot...

    end;

    I think you want to close *after* this `end;`.

    I sure do! :)

    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?

    That sure does catch the errors. I like using the 'try' block.

    At any rate, don't close your file while you're still reading it.

    Yup. It doesn't work as planned if you close the file while still reading
    it... <smacks forehead>

    Thank you for the help. I was looking in the wrong part of the code for the problem, and driving myself crazy...


    ---

    Black Panther
    a.k.a. Dan Richter
    Sysop - Castle Rock BBS (RCS)
    The sparrows are flying again....

    --- Mystic BBS v1.12 A38 2018/01/01 (Linux/64)
    * Origin: Castle Rock BBS - bbs.castlerockbbs.com (21:1/186)
  • From Black Panther@21:1/186 to tenser on Sat Jan 27 21:09:36 2018
    On 01/27/18, tenser said the following...

    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.

    I never thought about doing that. It would make the code more readable.

    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.

    Yes, 'corans' was set to false before this snippet, and right after this snippet is where it is set back to false. It does make sense to add this to
    the 'while' statement.

    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.

    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:

    snip<=-

    493.
    Who had a hit with this song?

    'Smooth Criminal'

    a. Guns N' Roses
    b. Michael Jackson
    c. Janet Jackson
    d. Simple Minds

    * B - Michael Jackson

    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.

    That make sense as well.

    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.

    end;
    close(trivia);

    Note that we close trivia *after* we terminate the outer loop.

    Got it! :)

    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.

    That's what I love about the 'try' 'except' blocks.

    Thank you so much. I really appreciate all of the assistance.


    ---

    Black Panther
    a.k.a. Dan Richter
    Sysop - Castle Rock BBS (RCS)
    The sparrows are flying again....

    --- Mystic BBS v1.12 A38 2018/01/01 (Linux/64)
    * Origin: Castle Rock BBS - bbs.castlerockbbs.com (21:1/186)
  • From tenser@21:1/112 to Black Panther on Sun Jan 28 08:46:53 2018
    On 01/27/18, Black Panther said the following...

    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.

    It comes from Dijkstra, and he was a very smart man. He was interested
    in predicate transformers; the use here may not be recognizable to him.

    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:

    Oh I see. And you have `y := '* ';` somewhere (I just noticed the
    first argument to `AnsiStartsStr` in this call is `y`, not `qstr`).
    In any case, the termination condition for this second loop should be:

    if AnsiStartsStr(y, linetmp1) then
    break;

    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.

    Actually...You know by the time you've reached this code that `linetmp1`
    must start with the string `* `. Given that, you know that there's at least
    one space. From looking at the documentation for `ExtractDelimited`, it
    looks like it doesn't handle multiple adjacent spaces. I think I'd just use `delete` here:

    (*
    * If we get here, `linetmp1` is the line corresponding to
    * our question that starts with `* `.
    *)
    Delete(linetmp1, 1, 2);
    if upcase(linetmp1) = upperx then
    corans := true;

    Thank you so much. I really appreciate all of the assistance.

    I'm happy to help!

    - Dan C.

    --- Mystic BBS v1.12 A38 2018/01/01 (Windows/32)
    * Origin: ACiD Telnet HQ / blackflag.acid.org (21:1/112)
  • From Jon Justvig@21:1/154 to tenser on Sat Jan 27 22:44:21 2018
    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? ;)

    Sincerely,
    Jon Justvig

    --- Mystic BBS v1.12 A38 2018/01/01 (Windows/32)
    * Origin: Raiders Inc BBS -- vintagebbsing.com (21:1/154)
  • From tenser@21:1/112 to Jon Justvig on Sun Jan 28 16:57:05 2018
    On 01/27/18, Jon Justvig said the following...

    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? ;)

    Heh; yeah, something very similar.

    --- Mystic BBS v1.12 A38 2018/01/01 (Windows/32)
    * Origin: ACiD Telnet HQ / blackflag.acid.org (21:1/112)