Please update the examples

[sametmax]

Hi,

While I like the concept of a this pattern, I feel like your examples are not reflecting its usefulness.

First, you introduce:

def get_user_name():
    current_user = request.user
    if current_user:
        return current_user.name

    return ''

But this is not idiomatic Python code, and any mid experienced dev would write it:

def get_user_name():
    return getattr(request.user, 'name', '')

Hence, it does not highlight any benefits of the maybe() version:

def get_user_name():
    return maybe(request.user).name.or_else('')

Then you have this second example:

stores = json.get('stores')
if stores:
    products = stores[0].get('products')
    if products:
        product_name = products[0].get('details', {}).get('name') or 'unknown'

And you replace it with:

product_name = maybe(stores)[0]['products'][0]['details']['name'].or_else('unknown')

I think this is misleading, as you remove the json.get() call you used in the first example and ignore the fact you would need to import maybe(). In reality, your exemple would read:

from pymaybe import maybe
stores = json.get('stores')
product_name = maybe(stores)[0]['products'][0]['details']['name'].or_else('unknown')

But then again, this would be against PEP8 because the line is 85 characters, which is too long. So a Python dev would write it this way:

from pymaybe import maybe
stores = json.get('stores')
product_name = maybe(stores)[0]['products'][0]['details']['name']
product_name = product_name.or_else('unknown')

Additionally, your first version is not how a Python dev would write it, he or she would most likely do:

try:
    product_name = json.get('stores')[0]['products'][0]['details']['name']
except (TypeError, KeyError, IndeError):
    product_name = 'unknown'

Again, this does not highlight the benefits of maybe(), and even worst, the maybe() version is actually more limited.

For once, it does not honor getitem:

    (test) >>> class Test:
        def __getitem__(self, value):
            return 1
          ...:     

    (test) >>> maybe(Test())[0]
    ---------------------------------------------------------------------------
    AttributeError                            Traceback (most recent call last)
    <ipython-input-6-e4a755e77e5b> in <module>()
    ----> 1 maybe(Test())[0]

    /home/sam/.local/share/virtualenvs/test/local/lib/python2.7/site-packages/pymaybe/__init__.pyc in __getitem__(self, key)
        212 
        213     def __getitem__(self, key):
    --> 214         return maybe(self.__value.get(key, None))
        215 
        216     def __setitem__(self, key, value):

    AttributeError: Test instance has no attribute 'get'

    (test) >>> Test()[0]
               1

And it doesn't play well with advanced collections such as defaultdict:

    (test) >>> maybe(defaultdict(datetime.utcnow))[0]
               None

    (test) >>> defaultdict(datetime.utcnow)[0]
               datetime.datetime(2015, 7, 30, 19, 39, 28, 298488)

I'm only saying this because I actually find the concept very interesting, and I'm looking actively for a useful usecase.

[ekampf]

Hey! Thanks for the comments! Looks like you really put some time into going over my code and I really appreciate it :)

I start my reply from the end because it was the easiest - your last 2 examples are indeed a bug and I'll look into fixing them ASAP. When implementing getitem in the Something class I assumes value would be a dict and didn't consider the case of someone overriding __getitem. As for defaultdict, it seems Something (and Nothing) are missing an implementation of missing.

Regarding the 2nd example: First, there's indeed a mistake in the doc as I unintentionally left the

stores = json.get('stores')

line from the maybe() example. However the fix should look like this (this statement should be pat of the maybe() call and not a separate one):

 from pymaybe import maybe
 product_name = maybe(json).stores[0]['products'][0]['details']['name'].or_else('unknown')

Or, if we're aiming for 85 characters line you can add a second line like in your code or use the \ notation.

Now, regarding the try..except block. You're right, a try..except would get the job done. But... what if you're not just trying to get _productname but 8 more values like it? You'll need 8 such blocks. Sometimes you can't even use try..except blocks - for example if your statement is inside a ninja template?

In those cases you would probably refactor your code to look somewhat like this:

 def get_value(getter, default):
    try:
         return getter()
    except (TypeError, KeyError, IndeError):
         return default

 product_name = get_value(lambda d=json: d['stores'][0]['products'][0]['details']['name'], 'unknown')
 product_desc = get_value(lambda d=json: d['stores'][0]['products'][0]['desc'], 'unknown')
 ...

Which is not bad. Thats pretty much what Maybe does for you...

Regarding the 1st example - its main purpose was to be a simple example of what Maybe does. I agree its not necessarily the best usecase as getattr is definitely nicer on this case. But its enough that we add a 2nd level to the object tree to make getattr not work. For example, if we wanted to do

 address = self.current_user.profile.address

Where both _currentuser and profile could be missing? You'll have to resort to try..except blocks again or a huge if statement...

Another strength of _orelse over getattr is that it accepts a callable - which is usable in case the default is not a value but an "expensive" calculation. A real world example would be an HTTP handler that has to receive a value from the client and if its not there go to DB to fetch a default. So we can do this:

my_value = self.request.get('my_value', calculate_default())

But then, _calculatedefault which is expensive always gets called. So we'll optimise:

my_value = self.request.get('my_value')
if not my_value:
   my_value = calculate_default()

Now _calculatedefault is only called if we didn't get a value from client. The same with maybe:

my_value = maybe(self.request)['my_value'].or_else(calculate_default)

Anyway, you're probably right and I need to improve the introduction section with better real world use-cases.

I will work on updating the docs and fixing the said bugs in the next couple of days.

Thanks a lot for the feedback! Eran

[ekampf]

Maybe a more real life example:

shipping_service_response = maybe(... call to some external service and get a json response ...) shipping_cost = shipping_service_response['shipping'][0]['price'].or_else(get_shipping_cost_default)

Which is much more straightforward and readable IMHO than:

def get_value(getter, default=None):
   try:
        return getter()
   except (TypeError, KeyError, IndexError):
        if callable(default):
            return default()

        return default

 shipping_service_response = ... call to some external service and get a json response ...
 shipping_cost = get_value(lambda: shipping_service_response['shipping'][0]['price'], get_shipping_cost_default)

Don't look at the length here, because obviously you can move _getvalue to a common utilities module and make this code 2 lines just like the maybe sample. But code\syntax wise I think the 1st one is nicer...

[ekampf]

FYI the 2 bugs you discovered resolved: https://github.com/ekampf/pymaybe/commit/4db54f3484c2a172a5c965f58e9078284ba07103

[sametmax]

Thank you for taking the time to improve the project.

As I said, I'm sincerely interested in it because I coded :

https://github.com/sametmax/minibelt/blob/master/minibelt.py#L299 https://github.com/sametmax/minibelt/blob/master/minibelt.py#L397

And I think your approach serves the same purpose, but in a more generic way. So I'll keep an eye on it and I'm considering replacing some of my existing toolkit with it.

[sametmax]

I think a better example would be:

from pymaybe import maybe
url = maybe(rss).load_feeds()[0]['url'].domain.or_else("planetpython.org")

This would be actually more annoying to do with only the stdlib:

try:
    url = rss.load_feeds()[0].url.domain
except (TypeError, IndexError, KeyError, AttributeError):
    url = "planetpython.org"

Especially since not writting down explicitly the errors would hide a bug that maybe() would not hide. So you need to think about all possible errors.

[ekampf]

Thats an excellent example! will update the README later today...