On 8 Mar, 06:02 pm, [EMAIL PROTECTED] wrote:
On Thu, Mar 08, 2007 at 06:54:30PM +0100, "Martin v. L?wis" wrote:
     back_name = splitext(name[0]) + '.bak'

back_name = splitext(name)[0] + '.bak'

This is really totally secondary to the point I actually care about, but seeing this antipattern bandied about as a reason to make the change is starting to bother me.

There's no way to "fix" this idiom. Forget the corner case; what if you have a foobar.py and a foobar.txt in the same directory? This is not at all uncommon, and your "backup" function here will clobber those files in any case.

Furthermore, although the module documentation is vague, the docstring for splitext specifically says "either part may be empty" and "extension is everything from the last dot to the end". Again, this is a case of needing a function designed to perform a specific task, and instead relying on half-broken idioms which involve other functions. make_backup_filename(x) might *use* splitext, but it is not what splitext is *for*. A correct implementation which did use splitext would look like this:

   def make_backup_filename(filename):
       base, extension = splitext(filename)
       return base + '.bak' + extension

although personally I would probably prefer this:

   def make_backup_filename(filename):
       return filename + '.bak'

If the behavior of the old code is going to be buggy in any case, it might as well be buggy and consistent. Consider a program that periodically makes and then cleans up backup files, and uses the "correct" splitext-using function above. Perhaps .cshrc.bak makes more sense than .bak.cshrc to some people, but that means that on a system where python is upgraded, .bak.cshrc will be left around forever. Even on a program whose functionality is improved by this change, the incompatibility between versions might create problems.
Python-Dev mailing list

Reply via email to