Hacker News Reader

Hacker News

Comment by bru | Hacker News Reader

Full thread

bru

Several of those patterns are incomplete or frowned upon:

* if a method does not use the object's state (no `self` usage) make it a `class-` or `staticmethod`.

* Some magic methods are presented. There's more to them[0].

* one should not write `class MyClass:` but `class MyClass(object):` (new style class[1])

* the last one (`return None`) make me very dubious

* Cascading methods: that's a big no. The idiom is that if a method may change the state of the object then it should return None (eg `set.add`)

0: well-written and comprehensive guide: http://www.rafekettler.com/magicmethods.html

1: http://www.python.org/doc/newstyle/

View on HN

For certain use-cases (like constructing queries for an ORM) or other things where you're effectively passing around curried ideas to eventually be executed, I think cascading methods is a huge win.

Yeah these things aren't as black and white as both the blogger and the original commenter make it seem. Software design is very subjective.

What you are referring to is a design pattern called fluent interfaces[1]. They do make for very usable APIs when used to represent pipelines and filters. They are also used heavily in creating domain specific language features. In your SQL example it works very well such as in SQLAlchemy. But in that example, the chained methods are building a query as opposed to mutating the actual data. Splitting hairs.

[1]http://en.m.wikipedia.org/wiki/Fluent_interface

> Splitting hairs.

Severely, since what you're doing is mutating the actual state-data of the query object.

In the case of SQLAlchemy you are not. The cascading methods on query objects create new query objects as it should be. Mutating objects with cascading methods is horrible API design as it suggests immutability where there is none.

Oh right, that's fair. Same in Django (and you're right, mutation in this case would be horrible design), I suppose I was mixing the metaphorical with the actual.

the API in some ways comes from that of Hibernate, which does actually mutate in place (see http://docs.jboss.org/hibernate/orm/3.3/reference/en-US/html...). I felt that keeping the existing object un-mutated is a lot more intuitive. I think the names that you choose for the methods do make a difference, e.g. Hibernate is saying "add()" which for whatever reason seems to imply "mutation" to me, not sure why.

Having used hibernate extensively (and considering many of it's features remain completely unmatched in any other ORM I've encountered), I can say that I strongly dislike the idea of mutating the query in place. There's huge advantages to each chain returning an independent and valid query that can be executed, referenced, and added to. I don't see any upside to the Hibernate approach here, and it violates "Prefer Immutability".

Showed this to a senior dev at work and he pointed out another (subtle) error

    for each in enumerate(items):
        if each.match(self):
            # This fails because enumerate yields an (index, value) tuple.
            # Match isn't a function defined on tuples.

a pretty basic error :(

Do you have to show the code to someone before your allowed to run it? Can't see how you wouldn't have spotted this yourself

> Cascading methods: that's a big no. The idiom is that if a method may change the state of the object then it should return None (eg `set.add`)

So methods that mutate state, and don't return None are un-Pythonic?

What about this example:

  a = [1, 2, 3]
  b = a.pop()

pop() mutates the state of a and returns a value that is not None. Is the Python code language itself un-Pythonic?

> * the last one (`return None`) make me very dubious

I'm curious, as someone who uses this pattern quite a bit, how would you improve on this?

Just speculating here, but a func that just ends without returning, effectively returns None, so that idiom may be considered redundant. IMHO, it's often worth the additional clarity to be redundant in this way.

    In [1]: def foo(): pass
    
    In [2]: foo() is None
    Out[2]: True

I was speculating about what the original complainer meant, not about what python does... b^)

Yes, but the "Pythonic" way to do things is that "explicit is better than implicit," so I'm not sure what the original poster considers wrong with `return None`.

I do this too. I dislike when functions in languages don't explicitly return something even if that something is nothing.

There are no procedures, only functions that return something. A function either returns something explicitly, or returns None. Easy.

That line and this one lead me to believe he's full of hot air:

> if a method does not use the object's state (no `self` usage) make it a `class-` or `staticmethod`.

Uh, that's a little too "cookbook" for my tastes. The decision of whether to make something a class / static method or not should be decided based on what the function is doing, rather than some pointless dogma. I'm assuming he means this method:

> def alarm(self): > with open(BUZZER_MP3_FILE) as f: > play_sound(f.read())

It's a design decision, and one that is particularly trivial at that. Let's say the effects an alarm ultimately change, resulting in an internal state read / write on the object. Now you have to go back and update the class despite the fact that it at a higher philosophical level one could argue that an alarm should only be triggered on an individual boiler.

Point being: it's nit-pickery for nit-pickery's sake. I don't personally use `return None`, but it's certainly more explicit and is fine to use if your judgment has found it to make sense.

It seems he's using Python 3 (using print as a function), so no need to inherit object.

There is a print function in Python 2 either!

http://docs.python.org/2/library/functions.html#print

These things turn subjective pretty quickly. That, and, "this is the way most people do it, so just do it this way."

Sometimes I find guidelines like these counterproductive.

For example, when looking through someone elses code, if it does not have deep levels of nesting I prefer longer methods you can read like a script rather than having to jump around the place to see what is each method.

Likewise with the 80 character guideline in PEP. It can take a lot longer to find the closing brace if it is hidden in columns that look like a newspaper article.

>Likewise with the 80 character guideline in PEP

Definitely. The PEP8 was actually revised 6 months ago[0]. Nothing crazy, but a bit more pragmatic.

>For code maintained exclusively or primarily by a team that can reach agreement on this issue, it is okay to increase the nominal line length from 80 to 100 characters (effectively increasing the maximum length to 99 characters)

0: text: http://legacy.python.org/dev/peps/pep-0008/#maximum-line-len...

commit : http://hg.python.org/peps/rev/fb24c80e9afb

Yes. I look at these things and pick out what I think are the good parts.

I think this is due to poor naming, not due to short functions.

Compare:

  def makeMove(self, target):
    enemy_piece = target.piece
    if enemy_piece:
      self.takeEnemyPiece(enemy_piece)
    target.piece = self
    self.position = target

And

  def changeCoords(self, foo):
    if foo.standing_on_it:
      self.nowItsMine(foo.standing_on_it)
    foo.standing_on_it = self
    self.coords = foo

In the former case, you mostly don't need to consult the source of functions called. In the latter, you're never sure.

Cascading methods aka fluid interface are OK in many cases, e.g. various builders. OTOH immutable objects are preferable in many cases.

What I really disliked is storing intermediate results of a multi-stage computation in instance variables. It is so easy to return multiple values from Python functions. Also, thinking what you need and need not pass and return between interconnected functions helps structure them much better, into smaller, clearer pieces (as #1 correctly advises).

> * the last one (`return None`) make me very dubious

simply `return` would be enough

What is your reasoning for not making methods chainable?

http://en.wikipedia.org/wiki/Fluent_interface#Java

Chainable mutating methods are not idiomatic Python; returning None is idiomatic for mutating methods. ISTR that the reasons here regard readability and clearly distinguishing mutations from queries, but in any case if you don't follow the idiom in libraries you create, they won't behave like Python's builtins and standard library, which, whatever you think about chaining on its own, will cause some context switching for most Python users when trying to work with it, increasing the cognitive load.

The builtins are broken, that is why there are new builtins liked `sorted()`, list.sort() returning None is not an improvement. The standard libraries are a _mess_, there is absolutely know cohesion in their api design, or even naming conventions (go peruse the standard lib). Much of what is Pythonic is someones opinion on how a nanny-language should behave.

At this point I think calling something idiomatic or Pythonic is a thought terminating statement. I need to see refutation.

sorted returns a new object, not mutates the original object, so it follows the convention GP is talking about. sort() also follows that rule, since it mutates, it returns None.

In terms of refutation, some things are just conventions, and breaking them has to be done with the understanding that the benefits outweigh the costs. If python were a clean slate, totally new language, you might have a discussion about the merits of mutating chaining, but as it is, the convention is not to do that, and the benefits don't buy enough to violate everyone's expectations.

Counterpoint, object.next() was removed from python3 and replaced with next(object). It both modifies state and returns data. I really prefered next() as a method since it does change the state of a generator.

> * if a method does not use the object's state (no `self` usage) make it a `class-` or `staticmethod`.

Is this really true?

Reply on news.ycombinator.com