[MERGE] bzrlib.osutils fixes

Andrew Bennetts andrew at canonical.com
Sat Mar 10 01:20:45 GMT 2007


Dmitry Vasiliev wrote:
[...]
> 
> Hmm, maybe I've misunderstood but:
> 
>     $ python -c "d = 'directory'; print d is 'directory'"
>     True

So long as the string looks like a Python identifier, and is a string literal
rather than e.g. read from a file, it will get interned by CPython so this will
be true.

It's not a feature of Python *the language* though, as I understand it, so this
would be a compatibility issue with running bzrlib in IronPython or Jython.

It's also more dangerous:

    python -c "d = 'directory'; print d is 'diretcory'"
    False

However if you misspell a variable name you pretty quickly get an exception
rather than a sometimes wrong answer masquerading as the right one.  Static
analysis tools like pychecker and pyflakes can also catch this typos in
identifiers, but have no way to know that a string constant is misspelled.

> so we don't lose anything without _directory_kind only gain some code 
> clearness. There is also code in osutils.copy_tree() which don't use 
> _directory_kind. Moreover I believe such a micro optimizations applied 
> to a real code doesn't gain much (if any at all) so code clearness is 
> more preferable.

By using a common name for the constant everywhere, you do gain some clarity,
though: when you see '_directory_kind' in the code, without needing further
context you know you can tell your editor to go to the definition of it, and
also discover the kinds and the purpose of this constant.  String literals don't
convey that information.  So it's clearer in one way, but less clear in another.

> So conclude I think we need to select one preferable way and follow it 
> consistently, so if we decide to leave the variable then corresponding 
> variables should be added for each file kind. Am I right?

I agree we should decide.  Personally, I think the constant would be better.

-Andrew.




More information about the bazaar mailing list