Hi Steve,
I've been away - as I mentioned on private mail - and getting back up to
speed. I did get a chance to read your mails whilst away, but not to
respond The key thing I've taken away from your comments is this:
> ... I have to tell you I'm really struggling
> with finding peace with the "Inheritable Default Values" approach.
... which I think is the strongest objection you had stylistically.
I think you get why I like the idea, and suspect you may also like this (esp
when you see the alternatives below...) :
class GreylistServer(ServerCore):
logfile = config["greylist_log"]
debuglogfile = config["greylist_debuglog"]
socketOptions=(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
port = config["port"]
class TCPS(TCPServer):
CSA = NoActivityTimeout(ConnectedSocketAdapter,
timeout=config["inactivity_timeout"],
debug=False)
class protocol(GreyListingPolicy):
servername = config["servername"]
serverid = config["serverid"]
smtp_ip = config["smtp_ip"]
smtp_port = config["smtp_port"]
allowed_senders = config["allowed_senders"]
allowed_sender_nets = config["allowed_sender_nets"]
allowed_domains = config["allowed_domains"]
whitelisted_triples = config["whitelisted_triples"]
whitelisted_nonstandard_triples = \
config["whitelisted_nonstandard_triples"]
... since it makes configuration/monkey patching a default, and structured
safely, and means you can end up writing less code.
BUT, the technique taken to get there has a cost you feel is too high. Which
is as you note a position I have some sympathy with.
> To boil it down, I can see this approach useful for optional
> parameters with defaults, but I think it sacrifices too much clarity
> for negligible gain when used for fundamentally necessary parameters.
You also won't be alone here, so I've had a think. (being away from being
able to reply has probably been helpful here, since I think I've reached a
similar conclusion to you)
Before I come to that, it's probably useful to reply to this:
> That said, it seems you came to a different internal equilibrium and I
> would really appreciate it if you could help me see it from your
> perspective.
... which takes us down the rabbit hole.
The idea was really to try it and see how it worked, what the practical
objections are, or whether doing things this way was nice.
I'll start off with TCPClient first, since it has to connect to something,
somewhere - otherwise it's not a generic TCPClient. It's also useful
because it hasn't currently been rewritten to taken advantage of
inheritable default values.
[ I'm not going to use the SingleTick example here because I don't
personally agree that you can't have a valid default delay, and I don't
want to get into that here :), and the views will cross over either way ]
TCPClient's init method currently looks like this:
class TCPClient(Axon.Component.component):
def __init__(self,host,port,delay=0, connect_timeout=60):
super(TCPClient, self).__init__()
self.host = host
self.port = port
self.delay=delay
self.CSA = None
self.sock = None
self.howDied = None
self.connect_timeout = connect_timeout
Whereas with inheritable defaults, for everything, it could be changed to:
class TCPClient(Axon.Component.component):
host = None
port = None
delay = 0
connect_timeout = 60
def __init__(self,**kwargs):
super(TCPClient, self).__init__(**kwargs)
self.CSA = None
self.sock = None
self.howDied = None
if self.host is None:
raise TypeError("__init__ requires a host argument")
if self.port is None:
raise TypeError("__init__ requires a port argument")
This then allows this:
class Port80Client_Declarative(TCPClient):
port = 80
class KamaeliaWebsiteClient_Declarative(Port80Client):
host = "www.kamaelia.org"
.. both without change to the __init__ structure. ie purely declarative
changes.
The alternatives are to create new factories explicitly like this:
def Port80Client_Factory(host, **args):
return TCPClient(host, 80, **args)
def KamaeliaWebsiteClient_Factory(**args):
return Port80Client("www.kamaelia.org", **args)
( I'm not recommending these BTW, just showing thought process. )
On a single level the rationale is less clear - Factory/Declarative are much
of a muchness. The issue is what happens when you go deeper inside
components that form larger systems. (ie something more "real")
In that scenario, greylister is probably the best example here:
class GreylistServer_DeclarativeVersion(ServerCore):
logfile = config["greylist_log"]
debuglogfile = config["greylist_debuglog"]
# socketOptions=(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
port = config["port"]
class TCPS(TCPServer):
CSA = NoActivityTimeout(ConnectedSocketAdapter,
timeout=config["inactivity_timeout"],
debug=False)
class protocol(GreyListingPolicy):
servername = config["servername"]
serverid = config["serverid"]
smtp_ip = config["smtp_ip"]
...
You can change this to the other form, but suppose it looks more like this:
def GreylistServer_FactoryVersion(config):
def customTCPServerFactory():
return TCPServer(
CSA = NoActivityTimeout(ConnectedSocketAdapter,
timeout=config["inactivity_timeout"],
debug=False)
)
def protocolFactory():
return GreyListingPolicy(
servername = config["servername"],
serverid = config["serverid"],
smtp_ip = config["smtp_ip"],
)
return ServerCore( logfile = config["greylist_log"],
debuglogfile = config["greylist_debuglog"],
# socketOptions=(socket.SOL_SOCKET, ..., 1),
port = config["port"],
TCPS = customTCPServerFactory,
protocol = protocolFactory)
Personally, I find that rather more convoluted - and also more fragile. The
latter version is very explicit about how things are made, but it's less
clear what happens if you, as a user of the Greylister adapt it.
I've deliberately commented out socketOptions in both of the examples above.
Now suppose in the former based entirely around inheritable defaults you want
to change this:
class GreylistServer_DeclarativeVersion(ServerCore):
.... as before ...
To support changing socket options, how do you do it? You 'just' do this:
class MyGreylistServer(GreylistServer_DeclarativeVersion):
socketOptions=(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
In the latter version, you either have to
* Copy all the code, from GreylistServer_FactoryVersion
* Change GreylistServer_FactoryVersion such that it supports this extra
option.
Which is less appealing - to me at any rate :)
You can also argue however, that the declarative approach is more inline with
the open closed principle of "Software entities (classes, modules, functions,
etc.) should be open for extension, but closed for modification."
With the factory approach, you actually have to take a copy of the code and
modify the copy, whereas with the declarative (inheritable defaults) approach
you can continue to extend the system, without modifying the original.
More subtley it also plays into the fact that the higher level you do in
Axon/Kamaelia, more of it is declarative, whereas as you go down inside
components they're OO/procedural.
Which is all well and good, BUT... practicality beats purity...
As you say here, this is relatively awkward if the most common case is going
to be simply using the TCPClient component, rather than inheriting from it...
And it is the most common case of course.
So this led me to this alternative set of possibilities:
class TCPClient(Axon.Component.component):
delay = 0
connect_timeout = 60
def __init__(self, host, port, **kwargs):
super(TCPClient, self).__init__(**kwargs)
self.CSA = None
self.sock = None
self.howDied = None
self.host = host
self.port = port
class Port80Client_Declarative(TCPClient):
port = 80
def __init__(self, host, **kwargs):
port = kwargs.pop("port", self.port)
super(Port80Client_Declarative, self).__init__(host, port, **kwargs)
class KamaeliaWebsiteClient_Declarative(Port80Client):
host = "www.kamaelia.org"
def __init__(self, **kwargs):
host = kwargs.pop("host", self.host)
super(KamaeliaWebsiteClient_Declarative, self).__init__(host, **kwargs)
...
... as being the most sensible alternative.
The only downside of all this really is that it prevents this:
TCPClient(host="www.kamaelia.org", port="80")
... which is nice, but not necessarily much of a loss.
I think based on this, most of my thoughts here are pretty compatible with the
changes you've made, but I'll have a detailed look tonight.
> but I'd like you to take a look at this way first.
I have indeed, and as you'll see I've taken your thoughts on board, and I
think reached a similar conclusion :-)
Apologies for the delay, but I really did need a holiday ;-) :-)
Regards,
Michael.
--
http://yeoldeclue.com/blog
http://twitter.com/kamaelian
http://www.kamaelia.org/Home
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"kamaelia" group.
To post to this group, send email to kamaelia@googlegroups.com
To unsubscribe from this group, send email to
kamaelia+unsubscr...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/kamaelia?hl=en
-~----------~----~----~----~------~----~------~--~---
