[MERGE REVIEW] create_checkout_convenience

[Martin Pool]

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
reader.
> > 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?

-- 
Martin