Re: cleaner way to write this try/except statement?
Simon Forman wrote: > Boris Borcic wrote: >> John Salerno wrote: >>> The code to look at is the try statement in the NumbersValidator class, >>> just a few lines down. Is this a clean way to write it? i.e. is it okay >>> to have all those return statements? Is this a good use of try? Etc. >>> >>> Thanks. >>> >>> >>> >>> import wx >>> >>> >>> class NumbersValidator(wx.PyValidator): >>> >>> def __init__(self): >>> wx.PyValidator.__init__(self) >>> >>> def Clone(self): >>> return NumbersValidator() >>> >>> def Validate(self, parent): >>> text_ctrl = self.GetWindow() >>> text = text_ctrl.GetValue() >>> >>> try: >>> if not text or int(text) <= 0: >>> wx.MessageBox('Enter a valid time.', 'Invalid time >>> entered', wx.OK | wx.ICON_ERROR) >>> return False >>> else: >>> return True >>> except ValueError, error: >>> wx.MessageBox('Enter a valid time.', 'Invalid time entered', >>> wx.OK | wx.ICON_ERROR) >>> return False >> well, assuming you are unsatisfied with the above, you could try to assert >> the >> validation condition and catch together all failures, eg : >> >> def Validate(self, parent): >> text_ctrl = self.GetWindow() >> text = text_ctrl.GetValue() >> try : >> assert int(text)>0 >> return True >> except (ValueError,AssertionError) : >> wx.MessageBox('Enter a valid time.', 'Invalid time entered', >>wx.OK | wx.ICON_ERROR) >> return False >> >> hth, BB > > Assertion statements "go away" when you run python with the '-O' or > '-OO' options. Makes me wonder how many use those options. I never do, this explains that. > They're only meant for debugging and shouldn't be used > as part of your actual program logic. That's too bad, and bound to evolve if the logic object space of Pypy gains users. > > You run the risk of introducing hard-to-find bugs if you use them like > this and somebody, somewhere, sometime runs your code in "optimized" > mode. Well, I wouldn't formulate it quite like that, but, as you say,... > > Peace, > ~Simon > Well, in that case let's just define def fail(exc) : raise exc and replace assert int(text)>0 by int(text)>0 or fail(ValueError) cheers, BB -- "On naît tous les mètres du même monde" -- http://mail.python.org/mailman/listinfo/python-list
Re: cleaner way to write this try/except statement?
John Salerno <[EMAIL PROTECTED]> wrote: > try: > if int(text) != 0: > return True > else: > self.error_message() > return False > except ValueError: > self.error_message() > return False One possible refactoring would be: try: if int(text) != 0: return True except: pass self.error_message() return False It's two less lines of code, one less return, and eliminates the duplicate call to error_message(). On the other hand, the "except: pass" clause is unusual and would make me (as a reader of the code) stop and think what was going on. Pick your poison. If you want to be truly cryptic and obfuscatory, there's always: try: 1 / int(text) return True except: return False -- http://mail.python.org/mailman/listinfo/python-list
Re: cleaner way to write this try/except statement?
John Salerno wrote: > The code to look at is the try statement in the NumbersValidator class, > just a few lines down. Is this a clean way to write it? i.e. is it okay > to have all those return statements? Is this a good use of try? Etc. Ok, I came up with this. It looks much nicer, I think, though I'm still unsure if it's good to have three separate return statements: def Validate(self, parent): text = self.GetWindow().GetValue() try: if int(text) != 0: return True else: self.error_message() return False except ValueError: self.error_message() return False But instead of a pop-up box for the error, I've decided to take Peter's advice and change the status text instead (plus color the input box). But here's an ultra OO newbie question: how do I refer to my frame's children from within the validator class? :) -- http://mail.python.org/mailman/listinfo/python-list
Re: cleaner way to write this try/except statement?
Simon Forman wrote: > Maybe I did miss something: Are you actually trying to make a static > method? Say, to be able to call it elsewhere than just the Validate() > method? I don't know if a static method is the best way to do it, but what I was trying to do was make a regular function within the validator class, so that I could call it whenever I needed to show that error message. I suppose I could just use a regular method to do it, too, but it seems more like a utility function than a method that requires 'self'. My identation seems correct to me (and I get no errors about it), but it's still not working the way I have it. Maybe I shouldn't mess with decorators right now anyway... -- http://mail.python.org/mailman/listinfo/python-list
Re: cleaner way to write this try/except statement?
Peter Otten wrote: > John Salerno wrote: > >> The code to look at is the try statement in the NumbersValidator class, >> just a few lines down. Is this a clean way to write it? i.e. is it okay >> to have all those return statements? Is this a good use of try? Etc. >> >> Thanks. >> >> >> >> import wx >> >> >> class NumbersValidator(wx.PyValidator): >> >> def __init__(self): >> wx.PyValidator.__init__(self) >> >> def Clone(self): >> return NumbersValidator() >> >> def Validate(self, parent): >> text_ctrl = self.GetWindow() >> text = text_ctrl.GetValue() >> >> try: >> if not text or int(text) <= 0: >> wx.MessageBox('Enter a valid time.', 'Invalid time >> >>entered', wx.OK | wx.ICON_ERROR) >> return False >> else: >> return True >> except ValueError, error: >> wx.MessageBox('Enter a valid time.', 'Invalid time entered', >>wx.OK | wx.ICON_ERROR) >> return False >> >> def TransferToWindow(self): >> return True >> >> def TransferFromWindow(self): >> return True > > Here's how I might do it: > > def is_positive_int_str(s): > try: > value = int(s) > except ValueError: > return False > return value > 0 > > class PositiveIntegerValidator(wx.PyValidator): > # ... > def Validate(self, parent): > text = self.GetWindow().GetValue() > if is_positive_int_str(text): > return True > wx.MessageBox(...) > return False > > Two usability notes: > - "Invalid time entered" is a confusing message when you actually want a > positive integer. > - Pop-up messages are a major annoyance. An alternative would be to change > the text color of the control and show a hint as to what input you expect > in a status bar. > > Peter > Thanks for all the help guys! I'm going to rework it a little and see what happens. Luckily, I'm also reading the chapter on exceptions in Python in a Nutshell, so maybe that will help solidify my understanding of how to construct them in this situation. -- http://mail.python.org/mailman/listinfo/python-list
Re: cleaner way to write this try/except statement?
John Salerno wrote: > The code to look at is the try statement in the NumbersValidator class, > just a few lines down. Is this a clean way to write it? i.e. is it okay > to have all those return statements? Is this a good use of try? Etc. > > Thanks. > > > > import wx > > > class NumbersValidator(wx.PyValidator): > > def __init__(self): > wx.PyValidator.__init__(self) > > def Clone(self): > return NumbersValidator() > > def Validate(self, parent): > text_ctrl = self.GetWindow() > text = text_ctrl.GetValue() > > try: > if not text or int(text) <= 0: > wx.MessageBox('Enter a valid time.', 'Invalid time > >entered', wx.OK | wx.ICON_ERROR) > return False > else: > return True > except ValueError, error: > wx.MessageBox('Enter a valid time.', 'Invalid time entered', >wx.OK | wx.ICON_ERROR) > return False > > def TransferToWindow(self): > return True > > def TransferFromWindow(self): > return True Here's how I might do it: def is_positive_int_str(s): try: value = int(s) except ValueError: return False return value > 0 class PositiveIntegerValidator(wx.PyValidator): # ... def Validate(self, parent): text = self.GetWindow().GetValue() if is_positive_int_str(text): return True wx.MessageBox(...) return False Two usability notes: - "Invalid time entered" is a confusing message when you actually want a positive integer. - Pop-up messages are a major annoyance. An alternative would be to change the text color of the control and show a hint as to what input you expect in a status bar. Peter -- http://mail.python.org/mailman/listinfo/python-list
Re: cleaner way to write this try/except statement?
Boris Borcic wrote: > John Salerno wrote: > > The code to look at is the try statement in the NumbersValidator class, > > just a few lines down. Is this a clean way to write it? i.e. is it okay > > to have all those return statements? Is this a good use of try? Etc. > > > > Thanks. > > > > > > > > import wx > > > > > > class NumbersValidator(wx.PyValidator): > > > > def __init__(self): > > wx.PyValidator.__init__(self) > > > > def Clone(self): > > return NumbersValidator() > > > > def Validate(self, parent): > > text_ctrl = self.GetWindow() > > text = text_ctrl.GetValue() > > > > try: > > if not text or int(text) <= 0: > > wx.MessageBox('Enter a valid time.', 'Invalid time > > entered', wx.OK | wx.ICON_ERROR) > > return False > > else: > > return True > > except ValueError, error: > > wx.MessageBox('Enter a valid time.', 'Invalid time entered', > > wx.OK | wx.ICON_ERROR) > > return False > > well, assuming you are unsatisfied with the above, you could try to assert the > validation condition and catch together all failures, eg : > > def Validate(self, parent): > text_ctrl = self.GetWindow() > text = text_ctrl.GetValue() > try : > assert int(text)>0 > return True > except (ValueError,AssertionError) : > wx.MessageBox('Enter a valid time.', 'Invalid time entered', >wx.OK | wx.ICON_ERROR) > return False > > hth, BB Assertion statements "go away" when you run python with the '-O' or '-OO' options. They're only meant for debugging and shouldn't be used as part of your actual program logic. You run the risk of introducing hard-to-find bugs if you use them like this and somebody, somewhere, sometime runs your code in "optimized" mode. Peace, ~Simon -- http://mail.python.org/mailman/listinfo/python-list
Re: cleaner way to write this try/except statement?
John Salerno wrote: > John Salerno wrote: > > The code to look at is the try statement in the NumbersValidator class, > > just a few lines down. Is this a clean way to write it? i.e. is it okay > > to have all those return statements? Is this a good use of try? Etc. > > I cleaned it up a little and did this, but of course this doesn't work. > Is this the wrong way to create a function (not method) within a class? > > > > def Validate(self, parent): > text_ctrl = self.GetWindow() > text = text_ctrl.GetValue() > > try: > if not text or int(text) <= 0: > error_message() > return False > else: > return True > except ValueError: > error_message() > return False > > @staticmethod > def error_message(): > wx.MessageBox('Enter a valid time.', 'Invalid time entered', >wx.OK | wx.ICON_ERROR) If you're not really interested in methods, static or otherwise, and actually just want a cleaner Validate() method, I would code it as follows (note that calling int() on an empty string will raise ValueError): def Validate(self, parent): text = self.GetWindow().GetValue() try: T = int(text) except ValueError: result = False else: result = T > 0 if not result: wx.MessageBox('Enter a valid time.', 'Invalid time entered', wx.OK | wx.ICON_ERROR) return result Peace, ~Simon -- http://mail.python.org/mailman/listinfo/python-list
Re: cleaner way to write this try/except statement?
John Salerno wrote: > The code to look at is the try statement in the NumbersValidator class, > just a few lines down. Is this a clean way to write it? i.e. is it okay > to have all those return statements? Is this a good use of try? Etc. > > Thanks. > > > > import wx > > > class NumbersValidator(wx.PyValidator): > > def __init__(self): > wx.PyValidator.__init__(self) > > def Clone(self): > return NumbersValidator() > > def Validate(self, parent): > text_ctrl = self.GetWindow() > text = text_ctrl.GetValue() > > try: > if not text or int(text) <= 0: > wx.MessageBox('Enter a valid time.', 'Invalid time > entered', wx.OK | wx.ICON_ERROR) > return False > else: > return True > except ValueError, error: > wx.MessageBox('Enter a valid time.', 'Invalid time entered', > wx.OK | wx.ICON_ERROR) > return False well, assuming you are unsatisfied with the above, you could try to assert the validation condition and catch together all failures, eg : def Validate(self, parent): text_ctrl = self.GetWindow() text = text_ctrl.GetValue() try : assert int(text)>0 return True except (ValueError,AssertionError) : wx.MessageBox('Enter a valid time.', 'Invalid time entered', wx.OK | wx.ICON_ERROR) return False hth, BB -- http://mail.python.org/mailman/listinfo/python-list
Re: cleaner way to write this try/except statement?
Here's how I would do it: def Validate(self, parent): try: text_ctrl = self.GetWindow() text = text_ctrl.GetValue() if not text: raise ValueError if int(text) <= 0: raise ValueError return True except ValueError, error: wx.MessageBox('Enter a valid time.', 'Invalid time entered', wx.OK | wx.ICON_ERROR) return False -- http://mail.python.org/mailman/listinfo/python-list
Re: cleaner way to write this try/except statement?
Simon Forman wrote: > John Salerno wrote: > > John Salerno wrote: > > > The code to look at is the try statement in the NumbersValidator class, > > > just a few lines down. Is this a clean way to write it? i.e. is it okay > > > to have all those return statements? Is this a good use of try? Etc. > > > > I cleaned it up a little and did this, but of course this doesn't work. > > Is this the wrong way to create a function (not method) within a class? > > > > > > > > def Validate(self, parent): > > text_ctrl = self.GetWindow() > > text = text_ctrl.GetValue() > > > > try: > > if not text or int(text) <= 0: > > error_message() > > return False > > else: > > return True > > except ValueError: > > error_message() > > return False > > > > @staticmethod > > def error_message(): > > wx.MessageBox('Enter a valid time.', 'Invalid time entered', > >wx.OK | wx.ICON_ERROR) > > Your indentation looks off. Your error_message() function should be at > the same indentation as the rest of the body of the Validate() method > (i.e. same as the try statement, "text_ctrl = ..." statements.) > > If that's not an artifact of posting it here then you'll need to > correct that. > > Also, there's no need to declare the function a staticmethod, since it > isn't. > > Other than that it looks ok to me, but I might have missed something. Maybe I did miss something: Are you actually trying to make a static method? Say, to be able to call it elsewhere than just the Validate() method? In that case, your indentation is still (apparently) off, but the error_message() method (a staticmethod is still a method I think, but I could be wrong) should be at the same indentation level as any other method, and you have to prepend the instance object ('self.' if you're calling it from inside another method) to it when you call it. class A(object): def meth(self, arg): self.func(arg) @staticmethod def func(arg): print arg a = A() a.func("hi") a.meth("world.") # prints hi world. HTH, ~Simon -- http://mail.python.org/mailman/listinfo/python-list
Re: cleaner way to write this try/except statement?
John Salerno wrote: > John Salerno wrote: > > The code to look at is the try statement in the NumbersValidator class, > > just a few lines down. Is this a clean way to write it? i.e. is it okay > > to have all those return statements? Is this a good use of try? Etc. > > I cleaned it up a little and did this, but of course this doesn't work. > Is this the wrong way to create a function (not method) within a class? > > > > def Validate(self, parent): > text_ctrl = self.GetWindow() > text = text_ctrl.GetValue() > > try: > if not text or int(text) <= 0: > error_message() > return False > else: > return True > except ValueError: > error_message() > return False > > @staticmethod > def error_message(): > wx.MessageBox('Enter a valid time.', 'Invalid time entered', >wx.OK | wx.ICON_ERROR) Your indentation looks off. Your error_message() function should be at the same indentation as the rest of the body of the Validate() method (i.e. same as the try statement, "text_ctrl = ..." statements.) If that's not an artifact of posting it here then you'll need to correct that. Also, there's no need to declare the function a staticmethod, since it isn't. Other than that it looks ok to me, but I might have missed something. Peace, ~Simon -- http://mail.python.org/mailman/listinfo/python-list
Re: cleaner way to write this try/except statement?
John Salerno wrote: > The code to look at is the try statement in the NumbersValidator class, > just a few lines down. Is this a clean way to write it? i.e. is it okay > to have all those return statements? Is this a good use of try? Etc. I cleaned it up a little and did this, but of course this doesn't work. Is this the wrong way to create a function (not method) within a class? def Validate(self, parent): text_ctrl = self.GetWindow() text = text_ctrl.GetValue() try: if not text or int(text) <= 0: error_message() return False else: return True except ValueError: error_message() return False @staticmethod def error_message(): wx.MessageBox('Enter a valid time.', 'Invalid time entered', wx.OK | wx.ICON_ERROR) -- http://mail.python.org/mailman/listinfo/python-list
cleaner way to write this try/except statement?
The code to look at is the try statement in the NumbersValidator class, just a few lines down. Is this a clean way to write it? i.e. is it okay to have all those return statements? Is this a good use of try? Etc. Thanks. import wx class NumbersValidator(wx.PyValidator): def __init__(self): wx.PyValidator.__init__(self) def Clone(self): return NumbersValidator() def Validate(self, parent): text_ctrl = self.GetWindow() text = text_ctrl.GetValue() try: if not text or int(text) <= 0: wx.MessageBox('Enter a valid time.', 'Invalid time entered', wx.OK | wx.ICON_ERROR) return False else: return True except ValueError, error: wx.MessageBox('Enter a valid time.', 'Invalid time entered', wx.OK | wx.ICON_ERROR) return False def TransferToWindow(self): return True def TransferFromWindow(self): return True class Timer(wx.Frame): def __init__(self): wx.Frame.__init__(self, None, wx.ID_ANY, 'Timer', style=wx.DEFAULT_FRAME_STYLE ^ wx.RESIZE_BORDER ^ wx.MAXIMIZE_BOX) self.panel = wx.Panel(self) mainSizer = wx.BoxSizer(wx.VERTICAL) inputSizer = wx.BoxSizer(wx.HORIZONTAL) self.progress = wx.Gauge(self.panel, wx.ID_ANY, 100, size=(300, 20)) self.status = wx.StaticText(self.panel, wx.ID_ANY, 'Enter a time.') prompt = wx.StaticText(self.panel, wx.ID_ANY, 'Time to wait:') self.input = wx.TextCtrl(self.panel, wx.ID_ANY, size=(20, 20), validator=NumbersValidator()) self.start = wx.Button(self.panel, wx.ID_ANY, 'Start') self.reset = wx.Button(self.panel, wx.ID_ANY, 'Reset') self.reset.Disable() self.sound = wx.Sound(r'C:\Windows\Media\notify.wav') self.timer = wx.Timer(self) mainSizer.Add(self.progress, flag=wx.ALIGN_CENTER | wx.ALL ^ wx.BOTTOM, border=10) mainSizer.Add(self.status, flag=wx.ALIGN_CENTER | wx.ALL, border=10) mainSizer.Add(inputSizer, flag=wx.ALIGN_CENTER | wx.BOTTOM, border=10) inputSizer.Add(prompt, flag=wx.ALIGN_CENTER) inputSizer.Add(self.input, flag=wx.ALIGN_CENTER | wx.LEFT | wx.RIGHT, border=5) inputSizer.Add(self.start, flag=wx.ALIGN_CENTER) inputSizer.Add(self.reset, flag=wx.ALIGN_CENTER) self.Bind(wx.EVT_TEXT_ENTER, self.OnStart, self.input) self.Bind(wx.EVT_BUTTON, self.OnStart, self.start) self.Bind(wx.EVT_TIMER, self.OnTimer, self.timer) self.Bind(wx.EVT_BUTTON, self.OnReset, self.reset) self.panel.SetSizer(mainSizer) mainSizer.Fit(self) def OnStart(self, event): if self.panel.Validate(): self.time = int(self.input.GetValue()) self.minutes_passed = 1 self.minutes_remaining = self.time - 1 self.start.Disable() self.input.Disable() self.reset.Enable() self.status.SetLabel('%s minute(s) remaining.' % self.time) self.timer.Start(1000) def OnReset(self, event): if self.timer.IsRunning(): self.timer.Stop() self.input.Clear() self.input.Enable() self.start.Enable() self.reset.Disable() self.status.SetLabel('Enter a new time.') self.progress.SetValue(0) self.minutes_passed = 1 def OnTimer(self, event): if self.minutes_remaining != 0: self.progress.SetValue(self.minutes_passed * 100 / self.time) self.status.SetLabel('%s minute(s) remaining.' % self.minutes_remaining) self.minutes_passed += 1 self.minutes_remaining -= 1 else: self.timer.Stop() self.progress.SetValue(self.minutes_passed * 100 / self.time) self.status.SetLabel('%s minute(s) have elapsed.' % self.time) wx.Sound.Play(self.sound) class MyApp(wx.App): def OnInit(self): frame = Timer() self.SetTopWindow(frame) frame.Show() return True if __name__ == '__main__': app = MyApp(False) app.MainLoop() -- http://mail.python.org/mailman/listinfo/python-list