ahopkins/mayim

[Feature Request] Allow `Optional` return type

Closed this issue · 4 comments

async def select_user_by_id(self, user_id: int) -> Optional[User]:
        ...

Is this meant to replace the raising of RecordNotFound?

I am not sure that I like this because to use it you now need to do this:

user = await executor.select_user_by_id(1234)
if not user:
    do_something()
else:
    do_something_else()

The explicit nature of knowing that select_user_by_id is ALWAYS and instance of User is much more usable. And then the catching of the exception can be abstracted into some more generic location of your application.

Is this meant to replace the raising of RecordNotFound?

Rather than replace the exception, I created this thinking about allowing for more flexibility. RecordNotFound could still be raised if the return type was not optional, whereas if it was optional then it would return None.

In my specific usecase, I want to check for the existence of something, and if it doesn't exist I raise a custom exception. While for me it currently works by making the query return a 0 or a 1 (ie: count(*)) and then marking the return type as bool, if I was to use the returned data for anything else it would look much cleaner simply doing an if check than writing several try...excepts.

You can see an example of how much cleaner this would look than a bunch of lines for exception handling here: https://github.com/prryplatypus/sanic-forum/blob/d18e52390017d68768bb869ae291de0313cccd58/sanic_forum/blueprints/api/v1/categories/blueprint.py#L29

Ahh, I think I see what you mean.

Basically do something like this:

  • if no result
    • if as_list:
      • return []
    • if allow_none:
      • return None
    • raise RecordNotFound
  • else return result

Yep, if allow_none means "Optional[...] as a return type", then that is indeed what I mean!