[PATCH] Factor out duplicate code

John Arbash Meinel john at arbash-meinel.com
Wed Oct 12 23:59:09 BST 2005


Robert Collins wrote:
> On Tue, 2005-10-11 at 18:45 -0700, Dan Loda wrote:
> 
>>... by adding a factory method to InventoryEntry.
>>
>>I'm new to Python; constructive criticism is most welcome ;)
>>
>>Great job on 0.1.
> 
> 
> Thanks for this Dan, I'll certainly merge in a variation.
> 
> In respect to the feedback, what I think has largely been overlooked is
> that the different types of entry need different information:
> 
> when someone is creating an entry from flat data - 'kind', 'id', 'parent
> id', 'name', often more data is available/needed - 'text_sha1',
> 'symlink_target' etc.
> 
> I think that the dictionary is an unuseful optimisation, because it does
> not take into account the differences - that a dir must not get a
> text_sha1 value and that a symlink can recieve a symlink_target
> parameter.
> 
> So - a single static method with a lot of parameters - yes, but with an
> if block.

Why not just have it be:

@staticmethod
def create(kind, file_id, *args, **kwargs):
  try:
    return InventoryEntry.versionable_kinds[kind](file_id,
	*args, **kwargs)
  except ???

Then you still have the dictionary to make it useful. And if the kind
doesn't take all the arguments given to it, then an exception will be
raised.

The idea of the dictionary is to let the information about creating an
inventory entry be distributed, so each individual entry knows how to
create itself, rather than having InventoryEntry.create() have to know
in advance about all types that might be created.

I really think a dictionary should almost always be used for factories.

John
=:->

> 
> Rob
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 256 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20051012/3f50df86/attachment.pgp 


More information about the bazaar mailing list