On Thu, Dec 05, 2019 at 05:40:05PM -0400, Juancarlo Añez wrote:
> I just found this code:
> 
> def get_product_item(jsonld_items):
>     for item in jsonld_items:
>         if item['@type'] == 'Product':
>             return item
>     else:
>         return {}

I'm sorry, I can't tell what that is supposed to do. Is the "return {}" 
supposed to be inside the loop? If so, it has been accidentally 
dedented. Is it meant to be outside the loop? The "for-else" is 
redundent, since there is no break.

    for item in jsonld_items:
        if item['@type'] == 'Product':
            return item
        else:  # Even this "else" is redundent too.
            return {}

    for item in jsonld_items:
        if item['@type'] == 'Product':
            return item
    # else is unnecessary here
    return {}

The name says it returns an item, but the default is to return an empty 
dict, which seems like an unusual choice for the "default product where 
no product is specified". I would have guessed None if the product is 
missing.


> My argument is that the intent is clearer in:
> 
> def get_product_item(jsonld_items):
>     return first((item for item in jsonld_items if item['@type'] ==
> 'Product'), {})

That's certainly an improvement, but the helper function "first" is 
redundant. You can just write:

    return next((item for item in jsonld_items if item['@type'] == 
                'Product'), default={})

What's the benefit of adding a new builtin which is essentially just a 
thin do-almost-nothing wrapper around `next`? Have I missed something?


-- 
Steven
_______________________________________________
Python-ideas mailing list -- python-ideas@python.org
To unsubscribe send an email to python-ideas-le...@python.org
https://mail.python.org/mailman3/lists/python-ideas.python.org/
Message archived at 
https://mail.python.org/archives/list/python-ideas@python.org/message/S5JG75H7DSQINDAPGZSKGVOLIQRFOESD/
Code of Conduct: http://python.org/psf/codeofconduct/

Reply via email to