[MERGE REVIEW] create_checkout_convenience
mbp at canonical.com
Mon Aug 7 10:17:00 BST 2006
On 7 Aug 2006, Aaron Bentley <aaron.bentley at utoronto.ca> wrote:
> Martin Pool wrote:
> > When talking to Robert the other day I proposed this rule: static/class
> > methods should be confined to the "named constructor" pattern.
> I'll have to get back to you about whether this should be a general
> rule. It seems strange to make things instance methods when they don't
> require access to instance variables.
(I should have explained what I meant by "named constructor", but Robert
did it well.)
Well, I certainly didn't mean to just turn them into instance methods
with no other changes. But there are other things that can be done:
1- Think of what instance they really should belong to, and put them there (as
we did in this thread).
2- Make a new class, and make them methods of instances of that class. For
example I think the register_format, set_default_format, etc should perhaps be
methods of an instance of FormatRegistry, rather than stuck into the class
whose subclasses they register.
3- Perhaps just make them regular functions, not part of any class. I know
Robert has concerns about the testability and maintainability of code that
does this. It's definitely the last choice but sometimes I think it makes
sense in Python. I can't really see any cases where it would be needed in
our current code so let's defer the abstract discussion. :-)
I agree it can be strange to have instance methods that don't care what
instance they're called upon. (Is there a name for that pattern?) However,
it can sometimes still make sense, if different subclasses would want to vary
the behaviour by instance. I guess in Python you still get instance-based
dispatch for @staticmethods, but it may communicate the wrong message to the
> > And also the 'lightweight' parameter basically runs two different
> > methods, which suggests we should actually have two. So how abut
> > source.create_lightweight_checkout(to_location, ...)
> The reasoning behind that was that we're trying to present lightweight
> checkouts and heavyweight ones as behaving essentially the same. At
> this level, I think if we make them separate methods, we make the client
> work harder, for no great gain. e.g. the implementation of cmd_checkout
> gets more complicated:
OK, so let's leave that for now and just make it an instance method?
More information about the bazaar