[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Object-oriented philosophy


On 2018-09-07 20:51, Michael F. Stemper wrote:
> On 2018-09-06 16:00, MRAB wrote:
>> On 2018-09-06 21:24, Michael F. Stemper wrote:
>>> On 2018-09-06 09:35, Rhodri James wrote:
> 
>>>> Is it worth creating the superclass in Python?? It sounds like it's a
>>>> bit marginal in your case.? I'm not that seasoned in object-oriented
>>>> design either, but my yardstick would be how much common code does it
>>>> eliminate?
>>>
>>> About half a dozen lines. Here's the common part:
>>>
>>> def __init__( self, xmlmodel, V, id ):
>>>
>>> ?? try:
>>> ???? P_0s = xmlmodel.findall( 'RatedPower' )[0].text
>>> ???? self.P_0 = float( P_0s )
>>> ?? except:
>>> ???? Utility.DataErr( "Power not specified for %s load" % (id) )
>>> ?? if self.P_0<=0.0:
>>> ???? Utility.DataErr( "Power for %s load must be postive, not %s" \
>>> ?????? % (id,P_0s) )
> 
>> A word of advice: don't use a "bare" except, i.e. one that doesn't
>> specify what exception(s) it should catch.
> 
> Given that I moved the first line ("P_0s = ...") out of the "try"
> clause, does this align with your advice?
> 
>   # If pre-validation has been run, guaranteed to have RatedPower
>   P_0s = xmlmodel.findall( 'RatedPower' )[0].text
>   try:
>     self.P_0 = float( P_0s )
>   except ValueError:
>     Utility.DataErr( "Power for %s load, '%s', not parsable" \
>       % (id,P_0s) )
>   if self.P_0<=0.0:
>     Utility.DataErr( "Power for %s load must be postive, not %s" \
>       % (id,P_0s) )
> 
That's better.

However, if P_0s can't be parse as a float, then self.P_0 will be 
unchanged (assuming that it already has a value, of course).

Is it OK that if there's no rated power, or it's invalid, it'll report 
it and continue with whatever value, if any, that it already has?

>> Your try...except above will catch _any_ exception. If you misspelled a
>> name in the 'try' part, it would raise NameError, which would also be
>> caught.
> 
> In another case where I had a "bare exception", I was using it to see if
> something was defined and substitute a default value if it wasn't. Have
> I cleaned this up properly?
> 
>   try
>     id = xmlmodel.attrib['name']
>   except KeyError:
>     id = "constant power"
> 
> (Both changes appear to meet my intent, I'm more wondering about how
> pythonic they are.)
> 
There's an alternative that's recommended when the key is often absent:

     id = xmlmodel.attrib.get('name', "constant power")

Catching an exception has a cost, so the usual advice is to use .get 
when the key is often absent, or catch KeyError when the key is usually 
present but occasionally is absent and you can provide a default value.