[PATCH] Factor out duplicate code

[John A Meinel]

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

I would not use "eval" I would instead create a dictionary mapping the 
kind to the actual python class. So you can do this:

    versionable_kinds = {}

    @staticmethod
    def versionable_kind(kind):
        return InventoryEntry.versionable_kinds.has_key(kind)

    @staticmethod
    def create(kind, file_id, name, parent_id):
        """Return an inventory entry of specified kind"""
        if not InventoryEntry.versionable_kind(kind):
            raise BadFileKindError("unknown kind %r" % kind)
        klass = InventoryEntry.versionable_kinds[kind]
        return klass(file_id, name, parent_id)

And then as you define them you do:

class InventoryDirectory(...):
   def __init__(...
   ...

InventoryEntry.versionable_kinds['directory'] = InventoryDirectory

In general, I always avoid using 'eval'. And in python a class 
definition is a first class object, so you can put it into a dictionary.

John
=:->


> 
> 
> ------------------------------------------------------------------------
> 
> === modified file 'bzrlib/branch.py'
> --- bzrlib/branch.py
> +++ bzrlib/branch.py
> @@ -713,15 +713,7 @@
>              name = os.path.basename(path)
>              if name == "":
>                  continue
> -            # fixme, there should be a factory function inv,add_?? 
> -            if kind == 'directory':
> -                inv.add(inventory.InventoryDirectory(file_id, name, parent))
> -            elif kind == 'file':
> -                inv.add(inventory.InventoryFile(file_id, name, parent))
> -            elif kind == 'symlink':
> -                inv.add(inventory.InventoryLink(file_id, name, parent))
> -            else:
> -                raise BzrError("unknown kind %r" % kind)
> +            inv.add(InventoryEntry.create(kind, file_id, name, parent))
>          self._write_inventory(inv)
>  
>      def unknowns(self):
> 
> === modified file 'bzrlib/inventory.py'
> --- bzrlib/inventory.py
> +++ bzrlib/inventory.py
> @@ -31,13 +31,12 @@
>  import types
>  
>  import bzrlib
> -from bzrlib.errors import BzrError, BzrCheckError
> +from bzrlib.errors import BzrError, BzrCheckError, BadFileKindError
>  
>  from bzrlib.osutils import (pumpfile, quotefn, splitpath, joinpath,
>                              appendpath, sha_strings)
>  from bzrlib.trace import mutter
>  from bzrlib.errors import NotVersionedError
> -
>  
>  class InventoryEntry(object):
>      """Description of a versioned file.
> @@ -277,9 +276,21 @@
>          l.sort()
>          return l
>  
> +    versionable_kinds = {'file': 'InventoryFile',
> +                         'directory': 'InventoryDirectory',
> +                         'symlink': 'InventoryLink' }
> +
>      @staticmethod
>      def versionable_kind(kind):
> -        return kind in ('file', 'directory', 'symlink')
> +        return InventoryEntry.versionable_kinds.has_key(kind)
> +
> +    @staticmethod
> +    def create(kind, file_id, name, parent_id):
> +        """Return an inventory entry of specified kind"""
> +        if not InventoryEntry.versionable_kind(kind):
> +            raise BadFileKindError("unknown kind %r" % kind)
> +        return eval(InventoryEntry.versionable_kinds[kind] +
> +                    '(file_id, name, parent_id)')
>  
>      def check(self, checker, rev_id, inv, tree):
>          """Check this inventory entry is intact.
> @@ -887,16 +898,8 @@
>          if parent_id == None:
>              raise NotVersionedError(parent_path)
>  
> -        if kind == 'directory':
> -            ie = InventoryDirectory(file_id, parts[-1], parent_id)
> -        elif kind == 'file':
> -            ie = InventoryFile(file_id, parts[-1], parent_id)
> -        elif kind == 'symlink':
> -            ie = InventoryLink(file_id, parts[-1], parent_id)
> -        else:
> -            raise BzrError("unknown kind %r" % kind)
> -        return self.add(ie)
> -
> +        return self.add(InventoryEntry.create(kind, file_id, parts[-1],
> +                                              parent_id))
>  
>      def __delitem__(self, file_id):
>          """Remove entry by id.
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 253 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20051011/57124c66/attachment.pgp