Python Forum

This is an opinion question.

When I code, I generally use assert statements, @property, or casting to check, error out, and/or correct improper parameters passed in. However, some feel this is bloat and I should just let bad parameters error out to force the calling developer to correct her/his code.

Which is the more pythonic method? Fix it or let it crash?

I.e.

def testMethod(intParam, stringCharParam):
    try: 
        intParam = int(intParam)
        if  ((intParam < 1) or (intParam > 10)):
            raise ValueError()
    except: 
        raise ValueError("The parameter intParam must be an integer between 1 and 10. [intParam = {}]".format(str(intParam)) )
    log.debug("intParam = {}".format(str(intParam)))

    stringCharParam = ''.join(c for c in str(stringCharParam) if re.match("[a-zA-Z]", c))
    assert len(stringCharParam) > 0 , "The parameter stringCharParam must be a string containing only characters (I.e. A-Z,a-z ). [stringCharParam = {}]".format(str(stringCharParam))
    log.debug("stringCharParam  = {}".format(stringCharParam ))

    print(intParam, stringCharParam)

Here's how I would do it:

def testMethod(intParam, stringCharParam):
    intParam = int(intParam)   # keep it simple, let int() raise it's own exception.
    if  ((intParam < 1) or (intParam > 10)):
        raise ValueError('Integer parameter must be between 1 and 10.')  # make your exceptions specific
    log.debug("intParam = {}".format(intParam))   # str() not needed, format handles that
 
    if not str(stringCharParam).isalpha():  # testing each character with a regex is total overkill
        raise ValueError('stringCharParam must be all letters.')   # assertion errors are rather vague, be specific
    log.debug("stringCharParam  = {}".format(stringCharParam ))
 
    print(intParam, stringCharParam)

Thanks for the code review and the example code (I'll offer an upvote tomorrow when my points reset :D )

What are your thoughts on the

stringCharParam = ''.join(c for c in str(stringCharParam) if re.match("[a-zA-Z]", c))
# or perhaps better ...
stringCharParam = ''.join(c for c in str(stringCharParam) if c.isalpha())

This would strip out all non-alpha chars and let the method continue, albeit with data different from what was entered. Some have felt this is bad, because if bad data comes in - the script "should" halt. It could also be bad, because the altered parameter may or may not hold the original intent.

I.e.
If the user enters "abc, " (abc comma space) by accident - then the corrections would reflect the correctly intended data (abc).

But if the user had entered "c2t0d0" (a Solaris disk path), the corrected value of "ctd" would continue processing sanely, but may result in completely incorrect results.

Messing with user input is tricky. Simple things, like capitalizing words is generally okay. Assuming things when the user expects the assumptions in generally okay, like expanding abbreviations. Stripping out the non-alphabetic characters seems to go too far.

How often are you going to get correct output having stripped characters vs. getting incorrect output? If it's a low probability of correct output, I would kick it back to the user rather than create a bunch of errors.

If stripping is the way to go, I might go with strCharParam = ''.join(re.findall('[A-Za-z]*', str(strCharParam))). I'm also a big fan of compiling regular expressions once and then reusing them.