ietf-tools / xml2rfc

Generate RFCs and IETF drafts from document source in XML according to the IETF xml2rfc v2 and v3 vocabularies
https://ietf-tools.github.io/xml2rfc/
BSD 3-Clause "New" or "Revised" License
63 stars 35 forks source link

refactor: Use context managers when opening files #1093

Closed kesara closed 5 months ago

kesara commented 5 months ago

lgtm

Off topic for the PR, but a couple of those might benefit from slight reorganization - instead of

  with open(...) as file:
    s = do_work()
    file.write(s)

you could refactor as

  s = do_work()
  with open(...) as file:
    file.write(s)

and that will have the benefit that a failure in do_work() won't leave a file around. That'd make sense if such a failure is more likely than a problem opening the file.

That's where context managers come in handy eh? It will guarantee that at the exit file will be closed regardless of failure happens during the execution of the do_work() or not.

Actually, there's also pathlib.Path(...).write_text(...) or write_bytes(...) that will do the latter without the with noise.

jennifer-richards commented 5 months ago

It will guarantee that at the exit file will be closed regardless of failure happens during the execution of the do_work() or not.

Right, the file handle will be closed, but it'll leave behind an empty file. Whether you care about that is a separate question, I'm just pointing out that you could avoid creating it until you know that you have data to put in it.

kesara commented 5 months ago

It will guarantee that at the exit file will be closed regardless of failure happens during the execution of the do_work() or not.

Right, the file handle will be closed, but it'll leave behind an empty file. Whether you care about that is a separate question, I'm just pointing out that you could avoid creating it until you know that you have data to put in it.

Good point. Yeah, this could be better.