morse-simulator / morse

The Modular OpenRobots Simulation Engine
http://morse-simulator.github.io/
Other
351 stars 155 forks source link

Corrected/improved the English + use of Python 3's super() #733

Closed mark-summerfield closed 7 years ago

mark-summerfield commented 7 years ago

Using super() is better practice (since it automatically works better with deeper inheritance).

Don't like the "overlay" terminology (sounds like an OpenGL graphics thing); prefer the term "function mapper" or "method mapper" but haven't changed it because such a change would have to be done throughout.

Also, if you're going to pass tuples about it is better to pass collections.namedtuples than plain tuples.

I also deleted the redundant \ line escapes in the second code example; you never need them inside (), [], or {}.

PierrickKoch commented 7 years ago

To come back on the use of super, one of Python's pillar is (from import this)

Explicit is better than implicit.

I personally feel that referencing explicitly the parent class is clearer. I think historically super was introduced for multiple inheritance, and we all know this is a nightmare.

It's just my "2 cents".

mark-summerfield commented 7 years ago

Hi Pierrick,

On Wed, 12 Oct 2016 18:13:53 -0700 Pierrick Koch notifications@github.com wrote:

To come back on the use of super, one of Python's pillar is (from import this)

Explicit is better than implicit.

I personally feel that referencing explicitly the parent class is clearer. I think historically super was introduced for multiple inheritance, and we all know this is a nightmare.

It's just my "2 cents".

I've finished on these now (just had a window of time and thought I might as well fix as I read).

The reason for prefering super() is that it makes refactoring easier if you put a new class between the original base class and the custom class, i.e.,

class Base:
...

class Derived(Base):
def __init__(self):
    Base.__init__(self)

Version #2

class Base:
...

class BaseExtra(Base):
...

class Derived(BaseExtra): # change #1
def __init__(self):
    BaseExtra.__init__(self) # change #2

Vs.

class Base:
...

class Derived(Base):
def __init__(self):
    super().__init__(self)

Version #2

class Base:
...

class BaseExtra(Base):
...

class Derived(BaseExtra): # only change
def __init__(self):
    super().__init__(self)

Anyway, just my 2c ;-)

Mark Summerfield, Qtrac Ltd. "Python in Practice" - Dr Dobbs JOLT best book award winner http://www.qtrac.eu/pipbook.html

severin-lemaignan commented 7 years ago

Documentation changes merged in c0b5854. For the discussion regarding the use of super(), please re-open a specific issue. Closing the PR.