mozilla / active-data-recipes

A repository of various activedata queries and recipes
Mozilla Public License 2.0
9 stars 24 forks source link

resolving comparison err #71

Closed MadinaB closed 6 years ago

MadinaB commented 6 years ago

Solving issue #70

MadinaB commented 6 years ago

What if this kind of null value would be returned for other queries? Would not it be a code duplication to check if element is null everywhere?

I am just wondering does it make sense to put that check in query.py right inside query_activedata? If we would not want to accept any null on any query from ActiveData it would make sense to do something like this:

    if check_null_in_response(json_response):
        raise MissingDataError("ActiveData returned null value.")

where check_null_in_response(json_response) could recursively divide json_response into primitive types and return true if any of them is null.

Something similar to:

def check_null_response(returned_value):
    result = false
    if returned_value is None:
        result = true
    if isinstance(returned_value, list) or  isinstance(returned_value, tuple):
       for item in returned_value:
           result = result or check_null_response(item):
    if isinstance(returned_value, dict):
        for item in returned_value.keys():
             result = result or check_null_response(item)
        for item in returned_value.values():
             result = result or check_null_response(item)
    return result
ahal commented 6 years ago

What if this kind of null value would be returned for other queries? Would not it be a code duplication to check if element is null everywhere?

Not a bad idea, but I think I'd prefer to keep this check local to this recipe. The reason is that here, a null value causes an exception to get raised, so isn't allowed.

However, some recipes might expect and deal with null values. So I think leaving it up to the recipe how to deal with null values is best.