3

Let's say we have a function that checks if a given pattern occurs in a string. The default behaviour is that if the pattern matches, then it returns the pattern. So far so good but what is the recommended way to handle the case where the pattern doesn't match. There are many alternatives of course but I can list two:

  • Return None and then handle the None from the calling environment
  • Raise (or throw) an Error (let's say NoParagraphNumberError) and handle the error from the calling environment.

In the following code, I return None but is it preferable to throw an Error, e.g. because it will increase readability, maintainability of the code or for any other reason?

import re

BASIC_CASE_PATTERN = r'\d{1,6}/\d+'
BASIC_PARAGRAPH_PATTERN = ur'(paras?(\W|\s)|paragraphs?)'


def check_case_citation(citation_string):
    """
    Check the citation string for occurence of patterns
    Returns:
        str: if just a case number is found
        None: if no case number is found or the citation includes paragraph numbers
    """
    paragraph = re.compile(BASIC_PARAGRAPH_PATTERN)
    casepattern = re.compile(BASIC_CASE_PATTERN)

    match = re.search(casepattern, citation_string)

    #1. entries with case number(s) but no paragraph given
    if match and re.search(paragraph, citation_string) is None:
        return match.group()
    else:
        return None


print check_case_citation("Case 145/80") # 145/80
print check_case_citation("Case 145/80 paragraph 3") # None
1
  • Migrated from Code Review because the question is about best practices in general rather than improving the code excerpt. – 200_success Apr 6 '16 at 20:36
5

I would definitely go for the return None option. Raising an exception may increase readability (although I doubt it) on the function itself, but handling it is messier. If you return None, from the caller function you can do the following:

citation = check_case_citation("Case 145/80")
if citation:
   # Do something
else:
   # Do something else

while, if you raise an exception, you would have something like

try:
   citation = check_case_citation("Case 145/80")
   # Do something
except NoParagraphNumberError:
   # Do something else

The third option, as suggested by Mathias Ettinger, would be to do

try:
   citation = check_case_citation("Case 145/80")
except NoParagraphNumberError:
   # Do something else
else:
   # Do something

I don't know about you, but to me the first alternative looks the cleanest and most straightforward...

8
  • 1
    In the case of the exception, I would definitely use the else clause of the try to do something and use a custom exception instead of the most generic one: try: citation = check_case_citation('Case 145/80'); except CheckException: do_handle_bad_case(); else: do_handle(citation); – 301_Moved_Permanently Apr 6 '16 at 20:01
  • @eSedano Thanks for your answer. Please see my edits. You are right about the Exception per se but I wanted to say Error instead, it is just that my Java past is haunting me :). So do you think your answer changes if we raise an Error? – Yannis P. Apr 6 '16 at 20:02
  • @MathiasEttinger do you mean except clause instead of else? – Yannis P. Apr 6 '16 at 20:05
  • 4
    @YannisP. No, I mean else. As in try .. except .. else .. finally. – 301_Moved_Permanently Apr 6 '16 at 20:07
  • @MathiasEttinger I have never come across this idiom. Strange logic but thanks for sharing. I wish I could upvote! – Yannis P. Apr 6 '16 at 20:13

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.