views:

265

answers:

7

I try to make my code fool-proof, but I've noticed that it takes a lot of time to type things out and it takes more time to read the code.

Instead of:

class TextServer(object):
    def __init__(self, text_values):
        self.text_values = text_values
        # <more code>
    # <more methods>

I tend to write this:

class TextServer(object):
    def __init__(self, text_values):
        for text_value in text_values:
            assert isinstance(text_value, basestring), u'All text_values should be str or unicode.'
            assert 2 <= len(text_value), u'All text_values should be at least two characters long.'
        self.__text_values = frozenset(text_values) # <They shouldn't change.>
        # <more code>
    @property
    def text_values(self):
        # <'text_values' shouldn't be replaced.>
        return self.__text_values
    # <more methods>

Is my python coding style too paranoid? Or is there a way to improve readability while keeping it fool-proof?

  • Note 1: I've added the comments between < and > just for clarification.
  • Note 2: The main fool I try to prevent to abuse my code is my future self.
+11  A: 

Here's some good advice on Python idioms from this page:


Catch errors rather than avoiding them to avoid cluttering your code with special cases. This idiom is called EAFP ('easier to ask forgiveness than permission'), as opposed to LBYL ('look before you leap'). This often makes the code more readable. For example:

Worse:

#check whether int conversion will raise an error
if not isinstance(s, str) or not s.isdigit:
    return None
elif len(s) > 10:    #too many digits for int conversion
    return None
else:
    return int(str)

Better:

try:
    return int(str)
except (TypeError, ValueError, OverflowError): #int conversion failed
    return None

(Note that in this case, the second version is much better, since it correctly handles leading + and -, and also values between 2 and 10 billion (for 32-bit machines). Don't clutter your code by anticipating all the possible failures: just try it and use appropriate exception handling.)

Eli Bendersky
You're right, but on the other hand it's good to fail as soon as possible. The values from the `text_values` frozenset property will be used later, so it will be harder to find out what's setting the property with invalid values.
Cristian Ciupitu
+9  A: 

"Is my python coding style too paranoid? Or is there a way to improve readability while keeping it fool-proof?"

Who's the fool you're protecting yourself from?

You? Are you worried that you didn't remember the API you wrote?

A peer? Are you worried that someone in the next cubicle will actively make an effort to pass the wrong things through an API? You can talk to them to resolve this problem. It saves a lot of code if you provide documentation.

A total sociopath who will download your code, refuse to read the API documentation, and then call all the methods with improper arguments? What possible help can you provide to them?

The "fool-proof" coding isn't really very helpful, since all of these scenarios are more easily addressed another way.

If you're fool-proofing against yourself, perhaps that's not really sensible.

If you're fool-proofing for a co-worker or peer, you should -- perhaps -- talk to them and make sure they understand the API docs.

If you're fool-proofing against some hypothetical sociopathic programmer who's out to subvert the API, there's nothing you can do. It's Python. They have the source. Why would they go through the effort to misuse the API when they can just edit the source to break things?

S.Lott
+3  A: 

It's unusual in Python to use a private instance attribute, and then expose it through a property as you have. Just use self.text_values.

Ned Batchelder
+3  A: 

Your code is too paranoid (especially when you want to protect only against yourself).

In Python circles, LBYL is generally (but not always) frowned upon. But there's also the (often unstated) assumption that one has (good) unit tests.

Me personally? I think readability is of paramount importance. I mean, if you yourself think it's hard to read, what will others think? And less readable code is also more likely to catch bugs. Not to mention making working on it harder/more time consuming (you have to dig to find what the code actually does in all that LBYLing)

jae
+2  A: 

If you try to make your code totally foolproof, someone will invent a better fool. Seriously, a good rule of thumb is to guard against likely errors, but don't clutter your code trying to think of every conceivable way a caller could break you.

dsimcha
Reminds me of "you can make something foolproof, but you can't make it damnfoolproof".
jae
Also, when you next time take a look at it, the future you definitely has become a more experienced fool and is way more devious at circumventing your proofings than the current you at creating them
Kimvais
+2  A: 

instead of spending time in assert and private variable i prefer spend time in documentation and test cases. i prefer read documentation and when i have to read code i prefer read tests. this is more true the more code grow up. in the same time tests give you a fool-proof code and useful use cases.

mg
+2  A: 

I base the need for error checking code on the consequences of the errors that it's checking for. If crap data gets into my system, how long will it be before I discover it, how hard will it be to determine that the problem was crap data, and how difficult will it be to fix? For cases like the one you've posted, the answers are generally "not long," "not hard," and "not difficult."

But data that's going to be persisted somewhere and then used as the input to a complicated algorithm in six weeks? I'll check the hell out of that.

Robert Rossney