I/O Error

[Michael Hoffman]

saif.shakeel at gmail.com wrote:

> file_input = raw_input("Enter The ODX File Path:")
> odx_file_output = raw_input("Enter the output file path : ")
> log_file_output = raw_input("Enter the path for LogFile  : ")
> 
> saveout = sys.stdout
> try:
>     f_open=open(odx_file_output, 'w')
> except:
>     print "cant open file"
>     sys.exit()
> sys.stdout = f_open
> 
> try:
>     input_xml = open(file_input,'r')
> except:
>     print "The File Cannot Be Opened"
>     sys.exit()
> 
> if input_xml.read(5)!='<?xml':
>     print "Invalid File"
>     input_xml.close()
>     sys.exit()
> else:
>     xmldoc = minidom.parse(input_xml)
>     input_xml.close()
> 
> if xmldoc.childNodes[1].getAttribute("DtdVers") == u'1.1.4' or
> xmldoc.childNodes[1].getAttribute("DtdVers")== u'1.1.5':
>     pass
> else:
>     print "Invalid Version"
>     sys.exit()
> 
>                     After this some more code follows,but i have
> pasted only the i/o part .

In the future, it is best to be able to produce a short and complete 
test case. Doing so may help you find your error, without assistance.

> Traceback (most recent call last):
>   File "C:\Projects\ODX Import\code_ini\odxparse.py", line 250, in
> <module>
>     file_input = raw_input("Enter The ODX File Path:")
> ValueError: I/O operation on closed file

Well, you set sys.stdout to f_open, and you probably closed it without 
setting it back. Can't tell because you didn't include the whole script 
(and you shouldn't do that either--make a test case).

Here are some general comments:

1) Redirecting sys.stdout does not seem advisable in this case, and 
seems to be causing some confusion. It's far better to make a new file 
handle for your output. You can print to it using:

print >>filehandle, "message"

2) Interactively asking for filenames like this will cause irritation 
for yourself, and possibly your users if they ever want to automate 
things. My personal preference would be to accept the arguments on the 
command line. If your users don't know how to use a command line, then 
you should really be getting the filenames through some sort of GUI 
instead of raw_input().

3) I try to name my file-related variables consistently so I know where 
they are. Naming the file name variable and the file handle variable 
something completely different is confusing.

4) If you have some abnormal exit condition, you should exit with 
sys.exit(1) or really any number besides 0. Exiting with sys.exit() or 
sys.exit(0) means everything is fine.

5) If an exception occurs that will result in the end of the program, 
there's no point in catching it just to print a less descriptive error 
message and quit. It makes your code harder to understand with all the 
exception catching, and it makes it harder to debug because you lose 
crucial details of where the exception occurred and what its calling 
stack was.

6) Further, I'm not sure how much sense it makes to double-check that 
the file begins with "<?xml". minidom.parse will check that just fine. 
And it will catch all sorts of other errors as well, and you can't do 
them all yourself at this point.

Here's how I would rewrite it using Python 2.5:

from __future__ import with_statement

import sys
from xml.dom import minidom

ACCEPTABLE_DTD_VERSIONS = [u'1.1.4', u'1.1.5']

class DTDVersionError(StandardError):
     pass

def io(infilename, outfilename, logfilename):
     with open(outfilename, "w") as outfile:
         with open(infilename) as infile:
             xmldoc = minidom.parse(input_xml)

     dtd_version = xmldoc.childNodes[1].getAttribute("DtdVers")
     if dtd_version not in ACCEPTABLE_DTD_VERSIONS:
         raise DTDVersionError(infilename)

def main(args):
     return io(*args)

if __name__ == "__main__":
     sys.exit(main(sys.argv[1:]))

As far as the interface goes, you can run this from the commandline as 
example.py INFILE OUTFILE LOGFILE. Or from IDLE as io("INFILE", 
"OUTFILE", "LOGFILE"). That way you can re-run it many times without 
having to retype three file names each time, yuck. If you want to add a 
GUI for other users to select files, you can call it from main() if 
there are no command-line arguments, keeping the bulk of your logic in 
io() separate from the interface.

Let us know if you have any questions about what I have done here.
-- 
Michael Hoffman