On Fri, 29 May 2015 01:01 am, Marko Rauhamaa wrote: > Anssi Saari <a...@sci.fi>: > >> Do you have an example of state pattern using nested classes and >> python? With a quick look I didn't happen to find one in any language. > > Here's an sampling from my mail server:
I haven't studied this in close detail, but first impressions is that this is not well-written Python code. The obvious problems that come to mind: > class SMTPServerConnection(ServerConnection): > #: : : > #: : : > > def __init__(self, server, sock, domain, peer): > super().__init__(server, sock) > conn = self > client_ip = peer[0] > > class STATE: > def __str__(self): > return self.__class__.__name__ A minor criticism: each time you instantiate a new SMTP server connection, it creates new STATE, IDLE etc. classes. That's wasteful and inefficient unless you have a good reason for it, although you may not care if you only have a single connection at a time. More serious problem: by nesting the state classes, it is much harder to test the state classes in isolation from a connection. > def handle_command(self, cmd): > conn.log("handle_command (unexpected): {}".format(cmd)) > assert False At first glance, this appears to be an abuse of `assert` and risks breaking your code when running under -O which turns debugging (and hence asserts) off. Since the intent is to make these abstract methods which must be overridden, I'd prefer to define an AbstractMethodError, then: raise AbstractMethodError("unexpected %s" % cmd) then do the logging elsewhere. This has the advantage(?) of eliminating the need to refer to conn, which means that the methods are no longer closures and the entire inner class can be moved out of the SMTPServerConnection body. [...] > class IDLE(STATE): > def handle_command(self, cmd): > conn.log("handle_command: {}".format(cmd)) Using a closure for information hiding is a solid functional equivalent to using private fields in an OOP context. However, "private" should be considered an anti-testing idiom. I would seriously consider this entire design to be an anti-pattern, and would prefer to one of two alternate designs: (1) Pull the state classes out of the SMTPServerConnection class. On initialisation, each state takes a conn argument. All references to "conn" are replaced by "self.conn". That's basically the conventional OOP approach. (2) Again, pull the state classes out of the SMTPServerConnection class, except this time there is no need to instantiate the state classes at all! Decorate all the methods with @classmethod, and have them take the connection as an explicit argument. The caller (namely the SMTPServerConnection instance) provides itself as argument to the state methods, like so: self.state = IDLE # No need to instantiate. self.state.process_ehlo_or_helo(self, other, args, go, here) This reduces implicit coupling, makes it explicit that the state methods depend on the connection, avoids reference loops, and enables easy mocking for testing. The only downsides are that people coming from a conventional (e.g. Java) OOP background will freak at seeing you using a class as a first class value (pun intended), and it will be ever-so-slightly tiresome to have to decorate each and every method with classmethod. (Perhaps a class decorator is the solution to that second issue?) The "design pattern" community is dominated by Java, where classes are not values, so this idiom is impossible in Java and unlikely to have a DP name. But it really should have one -- in a language where classes are themselves values, there is no reason why a class *must* be instantiated, particularly if you're only using a single instance of the class. Anyone ever come across a named design pattern that involves using classes directly without instantiating them? I'm basically looking for a less inelegant term for "instanceless class" -- not so much a singleton as a zeroton. -- Steven -- https://mail.python.org/mailman/listinfo/python-list