-1

We have a function that processes the user request if it is not None, how best to style it? I did not find a better choice in PEP8

Style 1

def user_handler(user):
   if db.getUser(user) is not None:
      """
      Here we have a big body of function
      """
      return {'message': 'Successful handled'}
   else:
      return {'message': 'User not found'}

Style 2

def user_handler(user):
   if db.getUser(user) is None:
      return {'message': 'User not found'}
   """
   Here we have a big body of function
   """
   return {'message': 'Successful handled'}


UPD 1: Since there were a lot of questions in the comments regarding the content of the code, I remade the question into a more real one.

RoyalGoose
  • 125
  • 5
  • 4
    Why would this handler be called with `None` as the request in the first place? The whole premise of this question is a code smell. – 200_success Mar 26 '22 at 23:31
  • 2
    We have a number of related questions, for example [Approaches for checking multiple conditions?](https://softwareengineering.stackexchange.com/q/191208). The point is, both approaches have their use. Personally, I strongly prefer guard clauses (get the edge cases out of the way before the main flow), don't like deeply nested logic, and prefer if decision and consequence (e.g. `return False`) are close together. In Style 1, Pylint will also flag the unnecessary `else`, since both branches `return`. – amon Mar 26 '22 at 23:34
  • 1
    @200_success I agree completely. When the handler returns a Boolean value the caller has to deal with this condition so it should have dealt with it before the call. – Hartmut Braun Mar 27 '22 at 12:44
  • @HartmutBraun, remaded question into real one – RoyalGoose Mar 27 '22 at 16:29

2 Answers2

4

I encourage using style no. 2.
The reason is, that an else after a short, concise early return statement is redundant noise. It adds no clarity to the code, since it is already obvious that a certain condintional branch was not taken due to the fact that the function is still running at this point, i.e. has not returned.
Especially in Python, a redundant else block also adds an unneccessary level of indentation which is also noise and reduces the usable line length without any reason.

Another possible issue with your function may be the big body you hinted to with the placeholder. Depending on the size of that block of code, the function might be doing too much and it may be a better idea to split the several processing steps into multiple functions dealing with part of the problem.

Richard Neumann
  • 203
  • 1
  • 7
2

There is no "best practice" answer to a question like this.*

My personal preference would be

def handler(request):
   if request is None:
      return False
   else:
      """
      Here we have a big body of function
      """
      return True

I like this better than your first option because the failure condition is at the top, which encourages reading the code in order. You're not getting to the end and then scrolling up to remember what the trailing else return False was for.

I like it better than your second option because when you get to the return True, you're still indented, which cues the reader that this isn't the only exit from the function. Equivalently, I like for every if to have an else that fills the same space in the program flow.

This is not a consensus opinion. Some people would prefer your second option because it doesn't "waste" a level of indentation. In other words, "quit early as needed, and then proceed confident of whatever assumptions you've ensured". My impression is that this the majority perspective among people who like python.

* certianly not in python; shout-out to haskell, where we could just lift to the Maybe monad.

ShapeOfMatter
  • 404
  • 3
  • 8
  • As a Javascript / NodeJS programmer who occasionally forgets to return correctly from a callback, I also use the redundant else after a return idiom. – user949300 Mar 27 '22 at 05:17
  • 3
    An else after a return is useless noise and produces unnecessary indentation within the following block. – Richard Neumann Mar 27 '22 at 09:11
  • @RichardNeumann OTOH, if one _additional_ level of indentation makes the code "too hard" to read, perhaps the code was too long and complex to begin with. Refactor it into a function or two. – user949300 Mar 28 '22 at 04:35