17 ms·

The more frequent and dangerous pitfalls are, in my humble opinion: - Bare except: statements (that catches everything, even Ctrl-C) - Mutables as default fun

• scott_w 12y ago
  Possibly the most interesting anti-pattern I saw was: a_list_of_words = "my list of words".split(" ") I never enquired why, since there were bigger issues in the code e.g. "unit testing" by running the code, taking the result and putting it as the check value. By running repr(value), copying out the string then comparing self.assertEqual(repr(value), '[<Object1: unicode_value>, ...]')
  • oddthink 12y ago
    I do that all the time in the interpreter, especially when slicing pandas DataFrame objects, e.g.: df_subset = df['date buyer nwidgets'.split()] That is far easier to type than the explicit list, with all its punctuation. Now, it's definitely weird that they did a `split(" ")` rather than just using the default, but the idea is the same. I do try to strip stuff like that out before I put it into a script, replacing it with the explicit list, but I'm never sure if that actually improves anything. It's not as if the explicit list is any easier to read.
    • klibertp 12y ago
      It's not weird, it's wrong: In [1]: "a string r".split(" ") Out[1]: ['a', '', '', '', 'string', '', '', '', '', 'r'] In [2]: "a string r".split() Out[2]: ['a', 'string', 'r']
      • glomph 12y ago
        In their case though that wouldn't have been a problem as each word was split on a single space.
      • hcarvalhoalves 12y ago
        Split without argument is equivalent to: >>> filter(None, " quick hack for split".split(" ")) ['quick', 'hack', 'for', 'split']
        • klibertp 12y ago
          Yeah, or re.split(r'\s+', ...), or something - the issue here is that you need to know about this. From the comment a few levels up I understood that the code which used the str.split with " " argument didn't signify that someone who written it knew about its semantics. If he did and it was really what was intended then ofc it's completely ok, but if not, it can easily lead to bugs. For example, if the user is required to input several ints separated with whitespace, this: map(int, input_str.split()) will rise only in expected cases, while this: map(int, input_str.split(" ")) can lead to rejecting correct input just because someone pressed space twice. It's very frustrating for the user, too, because whitespace are hard to spot visually. So, I don't know if this qualifies as antipattern, but I think if I saw .split(" ") instead of .split() in the code I'd at the very least expect the comment explaining why it's used.
    • blossoms 12y ago
      I don't mean to be pedantic, but a list (I am assuming df is a list) requires an int. (That sentence I wrote about using hashable types need not apply, sorry!) If you ran that code, you would get this error: TypeError: list indices must be integers, not list
      • gipp 12y ago
        That's a pandas dataframe (idiomatically denoted df), not a list. It has funky slicing properties, and he's selecting columns of the dataframe in a perfectly valid way.
  • coldtea 12y ago
    How is this a real "anti-pattern"? Inefficent, or bizarre way to do it maybe. Anti-pattern is supposed to mean something more, though. In this case there are no adverse effects and no ambiguity -- so, I guess the programmer was just lazy to construct the list.
  • bkeroack 12y ago
    Why even use a list here? Tuples are for immutable/constant data. a_tuple_of_words = ("my", "tuple", "of", "words") or a_tuple_of_words = "my", "tuple", "of", "words"
    • dozzie 12y ago
      ...because it's a list? Tuples were supposed to have a structure (at least that's what all the rest of the world thinks of them), so iterating through combination of apples, cars and languages makes no sense whatsoever. But yes, Python misses entirely the point of tuples, treating them as read-only lists. http://dozzie.jogger.pl/2014/04/11/python-tuples-the-useless-thing-in-language/ http://dozzie.jogger.pl/2014/04/11/python-tuples-the-useless...
      • bkeroack 12y ago
        I'm not sure what you're getting at here, or what you are expecting tuples to be like. They can have as much "structure" as you need--they're just a collection. Lighter-weight, immutable collections have a use case. The code in OP appears to be one where it makes sense. I follow the rule where variables are mutable IFF they need to be mutable.
        • dozzie 12y ago
          Tuples by pythonists are used as they were mere lists, just immutable. This is clearly displayed by Python's own interface. For the rest of the world, tuples are not immutable lists. They are tuples, i.e. collections of "objects" that could share nothing about their type. Tuples often are not even iterable! (Erlang, Haskell) The fact that tuples in Python can have as much structure as one wants is derived from dynamic typing, not from the tuples' nature. The same you could say about Python's lists. This is a really subtle issue. It takes to know more languages to see it clearly.
          • bkeroack 12y ago
            If it's so subtle, does it matter? This sounds like you just have a problem with the word "tuple" applied to an object that behaves differently from tuples in a statically-typed language. Would you feel better if they named it "ImmutableList" instead?
  • james2vegas 12y ago
    perhaps that line was written by someone used to Perl, where they would have had @a_list_of_words = qw/my list of words/; there
    • dozzie 12y ago
      Or rubyst: a_list_of_words = %w{my list of words}
    • rbonvall 12y ago
      You'd be sure if they wrote: >>> qw = str.split >>> qw('my list of words') ['my', 'list', 'of', 'words']
  • bluecalm 12y ago
    This is actually useful: you may want to experiment with different list_of_words in the future and typing the words between [" ", " ", " "] is time consuming. It's also less readable.
• cabalamat 12y ago
  > Mutables as default function/method arguments It would really make sense to change the semantics of Python to fix this issue.
  • klibertp 12y ago
    Remove a `static` keyword from C (used inside a function) while you're at it...
  • bkeroack 12y ago
    Change them how, to no longer have functions be first class objects? The behavior of mutable default arguments is clear if you know how Python treats function objects. Any "fix" would handicap the language.
    • jcampbell1 12y ago
      Can you clarify? def foo(default_arg = []): Why can't that just be shorthand for: def foo(default_arg = ParamNone): if default_arg == ParamNone: default_arg = [] How would that break first class functions?
      • bkeroack 12y ago
        How would the above work for duck-typed lists, for example? How would the language "know" which types are mutable?
      • dalke 12y ago
        As a minor point, use "default_arg is ParamNone", since "==" probably won't do the right thing. What breaks is something like: def foo(default_arg = slow_f()): pass Under the shorthand gets turned into: ParamNone = object() def foo(default_arg = ParamNone): if default_arg is ParamNone: default_arg = slow_f() pass This is fine, since everyone would know that the shorthand means to not put slow code there. Instead, people will start writing it as: _foo_arg = slow_f() def foo(default_arg = _foo_arg): pass Of course, then what happens with: _foo_arg = slow_f() def foo(default_arg = _foo_arg): _foo_arg = 5 ? Under expansion it becomes: _foo_arg = slow_f() def foo(default_arg = ParamNone): if default_arg is ParamNone: default_arg = _foo_arg _foo_arg.add(5) This violates Python's scoping rules, because _foo_arg is now being used in local scope instead of global scope. Eg: >>> def f(x=None): ... if x is None: ... x = spam ... spam = 3 ... >>> spam = 9 >>> >>> f() Traceback (most recent call last): File "<stdin>", line 1, in <module> File "<stdin>", line 3, in f UnboundLocalError: local variable 'spam' referenced before assignment Which means you now need a new scoping rule, just to handle default parameters without making things more confusing. It also turns what was a simple O(1) offset into a precomputed list into a globals() lookup for many cases.
• peter-row 12y ago
  Also, not knowing what scope a variable has when creating closures.
• SEJeff 12y ago
  Couldn't agree more! One of my all time new python interview questions gets a surprisingly large number of developers. Given a function like: def append_one(l=[]): l.append(1) return l What does this return each time? >>> append_one() >>> append_one() >>> append_one()
  • ryangittins 12y ago
    The l (lowercase L) and the 1 (one) look really similar. Could that be the cause of some confusion? Of course, the function name helps, but most developers have learned not to trust function names to be an accurate description of what the function does, especially in tricky interview questions. Still, I'd change this to something like: def append_five(l=[]): l.append(5) return l It tests the same thing (knowledge of how default parameters work), but without the confounding problem of similar-looking characters. Of course, syntax highlighting would help the applicant out. All of that being said, I still don't doubt that many developers don't know what they should about default parameters.
    • trevor-e 12y ago
      I'm not a Python expert, but iirc from various blog posts the "l" variable does not get reset between function calls which will cause undesired behavior. So calling the function 3 times without argument would produce a list of size 1,2, and 3 with the third call rather than 3 lists of size 1. Can any Python guru's confirm?
      • ryangittins 12y ago
        Yes, that's correct. The default value is only interpreted once, when the `def` statement is called. After that point, it's completely mutable. You have to see Python functions as objects and default parameter values as object variables.
        • Dylan16807 12y ago
          The problem is not the existence of the object variables, but that they are in such an unfortunate place. The rest of the parameter list is declaring fresh local variables. It's inconsistent that the left side of the equals sign is per-invocation, and the right side is per-def.
• geertj 12y ago
  Agreed on all counts. However I do find myself using mutables as default arguments sometimes because the generated documentation is clearer. For example, this is a real method in one of my projects: def listen(self, address, ssl=False, ssl_args={}): pass I like the way this turns up in the docs because it's immediately clear that ssl_args needs to be a dict. Otherwise I have to describe it in words.
  • tribaal 12y ago
    Well, that's throwing people implementing subclasses under the bus, IMO. Why not just add @param annotations in your docstrings instead?
    • geertj 12y ago
      > Well, that's throwing people implementing subclasses under the bus, IMO. If they need to touch this argument in an overridden method and they don't know what they are doing, then yes. > Why not just add @param annotations in your docstrings instead? I'm using Sphinx and it renders them separately. I want the empty dict to show up in the function signature.
      • Peaker 12y ago
        A single cryptic bug due to this practice will more than negate the minor doc readability benefits you get from that. And there will likely be many more than one cryptic bug. There are other ways to emphasize it ought to be a dict/mappable. Change its name to be suffixed as "_dict", for example?
  • jamiis 12y ago
    You should not ever use mutable default args. The documentation benefit is drastically outweighed by the potential for nasty bugs. http://pythonconquerstheuniverse.wordpress.com/2012/02/15/mutable-default-arguments/ http://pythonconquerstheuniverse.wordpress.com/2012/02/15/mu...
  • kunstmord 12y ago
• doctoboggan 12y ago
  I agree that bare excepts are bad, however they do not catch Ctrl-C. If you _do_ want to catch Ctrl-C you have to except a KeyboardInterrupt explicitly.
  • njharman 12y ago
    This is untrue (just tested) for at least Python 2.7.x. under Linux, try ... except catches Ctrl-C.
• LyndsySimon 12y ago
  Wildcard imports are OK, if and only if the module is specifically designed with this usage in mind. Example: nose.tools