1

I'm just starting on Python and maybe I'm worrying too much too soon, but anyways...

log = "/tmp/trefnoc.log"

def logThis (text, display=""):
    msg = str(now.strftime("%Y-%m-%d %H:%M")) + " TREfNOC: " + text
    if display != None:
        print msg + display
    logfile = open(log, "a")
    logfile.write(msg + "\n")
    logfile.close()
    return msg

def logThisAndExit (text, display=""):
    msg = logThis(text, display=None)
    sys.exit(msg + display)

That is working, but I don't like how it looks. Is there a better way to write this (maybe with just 1 function) and is there any other thing I should be concerned under exiting?


Now to some background (but not about trefnoc)...

Sometimes I will call logThis just to log and display. Other times I want to call it and exit. Initially I was doing this:

logThis ("ERROR. EXITING")
sys.exit()

Then I figured that wouldn't properly set the stderr, thus the current code shown on the top.

My first idea was actually passing "sys.exit" as an argument, and defining just logThis ("ERROR. EXITING", call=sys.exit) defined as following (showing just the relevant differenced part):

def logThis (text, display="", call=print):
    msg = str(now.strftime("%Y-%m-%d %H:%M")) + " TREfNOC: " + text
    call msg + display

But that obviously didn't work. I think Python doesn't store functions inside variables. I couldn't (quickly) find anywhere if Python can have variables taking functions or not! Maybe using an eval function? I really always try to avoid them, tho. Sure I thought of using if instead of another def, but that wouldn't be any better or worst.

Anyway, any thoughts?

Community
  • 1
  • 1
cregox
  • 17,674
  • 15
  • 85
  • 116
  • Tho I accepted an answer, I'm still missing some information for the **stderr** part. But unless someone says something, I'm just assuming there's nothing more that could be done on that side. – cregox Apr 14 '10 at 01:07
  • `sys.stdout` is where `print` generally outputs, `sys.stderr` is where exceptions output, but you can use `print` with it by going `print >> sys.stderr, 'blah'` if that's what you mean – cryo Apr 14 '10 at 02:02

5 Answers5

2

There's no reason for "logThisAndExit", it doesn't save you much typing over

sys.exit(logThis(text)+display)

(compare logThisAndExit(text, display))

or

sys.exit(logThis(text))

(compare logThisAndExit(text))

Not that I'm entirely sure why you like your exit messages formatted as log lines.

In answer to your original question: you're missing parentheses: call(msg+display) works fine. But I think that's waaaay overengineering for logging/exiting stuff. Anyone who maintains your code will have to understand your function to know when it's exiting and when it's not.

moshez
  • 36,202
  • 1
  • 22
  • 14
  • 1
    Well, it's just for the looks. It hits much better on the eye to write `logThisAndExit("ERROR, EXITING.")` than ` sys.exit(logThis("ERROR, EXITING", display=None) + "ERROR, EXITING")`. But this helped me a lot of thinking and I'm also not entirely sure if I want my exit messages formatted as log. Thanks! :D – cregox Apr 14 '10 at 00:50
  • Oh, and reason why call wasn't working is pointed in the other answer - `call=print` in the argument couldn't be done. If that was right, then I'd have an issue with `call msg+display` as well, as you pointed out, tho. – cregox Apr 14 '10 at 01:02
  • @Cawas - speaking as a long-term Python developer, I tend to agree with moshez that `logThisAndExit` is unidiomatic. – Charles Duffy Apr 14 '10 at 01:09
2

For logging, it is probably easier to use the logging module.

For exiting, if you have any error, use:

sys.exit(1)

and if there is no error, either just let the script run out of statements or:

sys.exit(0)
blokeley
  • 6,726
  • 9
  • 53
  • 75
  • well, what about putting the error message under `sys.exit`, like I did? is that a 1 or 0? – cregox Apr 14 '10 at 14:28
  • The quickest way to find out is to test it. Just run your app from the command line and then see what the return code was (` echo $?` on unix/linux or `echo %ERRORLEVEL%` on Windows). – blokeley Apr 15 '10 at 08:25
1

You could modify logThis to take a final argument called shouldExit which defaults to None, then as a final step in that method, if the value is true then call sys.exit.

maerics
  • 151,642
  • 46
  • 269
  • 291
1

print is a keyword, not a function, in python < 3. try this:

def do_print(x):
    print x

def logThis (text, display="", call=do_print):
    msg = str(now.strftime("%Y-%m-%d %H:%M")) + " TREfNOC: " + text
    call(msg + display)

Is there any reason you don't use the logging module? (see http://onlamp.com/pub/a/python/2005/06/02/logging.html)

cryo
  • 14,219
  • 4
  • 32
  • 35
  • I'm on python 2.6.4 and that worked quite well, thanks! No reason for not using the module. I was just introduced for it and will take a look, so, thanks again! ;) – cregox Apr 14 '10 at 00:47
  • Pending output is supposed to flush automatically whenever files are gracefully closed, which includes non-anomalous exits (excluding some corner cases involving threads which aren't shut down gracefully). Can you demonstrate the necessity of that `time.sleep(1)` with a modern interpreter? – Charles Duffy Apr 14 '10 at 01:07
  • Looks like you found a bug in *my* code :-) I was using a logger which ran in a separate thread so it didn't lock up the interpreter and trims the length of lines as Eclipse raises an exceptions if the lines are longer than a certain point, that must have been it – cryo Apr 14 '10 at 01:21
0

Just as reference, this is my final code after assimilating hints from David and moshez. In the end I decided I wanted just 1 function for now. Thanks everyone!

log = "/tmp/trefnoc.log"

def logThis (text, display=""):
    msg = str(now.strftime("%Y-%m-%d %H:%M")) + " TREfNOC: " + text
    if display != None:
        print msg + display
    logfile = open(log, "a")
    logfile.write(msg + "\n")
    logfile.close()
    return msg

# how to call it on exit:
sys.exit(logThis("ERROR, EXITING", display=None))
cregox
  • 17,674
  • 15
  • 85
  • 116