sa**********@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
