git.net

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

Object-oriented philosophy


On 2018-09-06 09:34, Marko Rauhamaa wrote:
> "Michael F. Stemper" <michael.stemper at gmail.com>:
> 
>> Since the three classes all had common methods (by design), I
>> thought that maybe refactoring these three classes to inherit from
>> a parent class would be beneficial. I went ahead and did so.
>> (Outlines of before and after are at the end of the post.)
>>
>> Net net is that the only thing that ended up being common was the
>> __init__ methods. Two of the classes have identical __init__
>> methods; the third has a superset of that method. The other methods
>> all have completely different implementations. This isn't due to
>> poor coding, but due to the fact that what these model have
>> different physical characteristics.
>>
>> [...]
>>
>> Is there really any benefit to this change? Yes, I've eliminated
>> some (a few lines per class) duplicate code. On the other hand,
>> I've added the parent class and the (probably small, but not
>> non-existent) overhead of invoking super().
>>
>> How does one judge when it's worthwhile to do this and when it's
>> not? What criteria would somebody seasoned in OO and python use
>> to say "good idea" vs "don't waste your time"?
> 
> Ultimately, a seasoned OO programmer might prefer one structure in the
> morning and another structure in the afternoon.

I figured that was the case, which is why I sought criteria to consider
rather than a cut-and-dried answer.

> Python's ducktyping makes it possible to develop classes to an interface
> that is not spelled out as a base class.

After reviewing my code, I have a feeling that I'm not taking
advantage of duck typing. I think that if LoadList held all types
of load and I did this:

for load in LoadList:
  load.served = load.Power( Feeder.V_load )
  LoadServed += load.served

it would be duck typing. Unfortunately, I'm not. I had thought that
I needed to handle the constant power load (if there is one)
separately in at least one point, and the same for a possible
constant current load. The (zero to many) constant impedance loads
are all in a list over which I loop.

However, inspired by your point, I think that I've found a way to
handle them all in one list. I'll play with it a little.

>   My opinion is that you should
> not define base classes only to show pedigree as you would need to in,
> say, Java.

Maybe it was from what I've read about Java (zero experience) that gave
me the idea to try this.

> Then, is it worthwhile to define a base class just to avoid typing the
> same constructor twice? That's a matter of opinion. You can go either
> way. Just understand that the base class doesn't serve a philosophical
> role but is simply an implementation utility. Be prepared to refactor
> the code and get rid of the base class the moment the commonality
> disappears.

I'll keep that in mind, but the commonality is based on the laws of
physics, so it's not a big risk.

> On the other hand, why is it that the two constructors happen to
> coincide? Is it an indication that the two classes have something deeper
> in common that the third one doesn't? If it is not a coincidence, go
> ahead and give the base class a name that expresses the commonality.

The code which all three load types share, which is the total content
of their parent, is as follows:

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) )


The constant impedance loads then extend this by initializing a
duty cycle model. (Think of something thermostatically controlled,
such as an electric oven or electric space heating.)

The constant impedance loads then extend this by initializing a
duty cycle model. (Think of something thermostatically controlled,
such as an electric oven or electric space heating.)

The 30000-foot[1] view of the data model is:

<!ELEMENT Scenario (Feeder,PLoad?,ILoad?,ZLoad*)>
<!ELEMENT PLoad (RatedPower)>
<!ELEMENT ILoad (RatedPower)>
<!ELEMENT ZLoad (RatedPower,DutyCycle)>
<!ELEMENT DutyCycle (Period,FractionOfTimeOn,TimeSinceLastTurnedOn)>

> Don't be afraid of "wasting your time"

If was was afraid of that, I wouldn't have tried refactoring the code
in the first place. I would have said "it works, don't bother."

>   or worry too much about whether
> something is a "good idea."

Well, if it turns out to be a bad idea, I'd rather learn that it's a
bad idea.

>   Be prepared to massage the code over time as
> your understanding and tastes evolve.

Exactly what I was doing, and the point of asking was to extend my
understanding.

Thanks for taking the time to respond.

-- 
Michael F. Stemper
What happens if you play John Cage's "4'33" at a slower tempo?