2
\$\begingroup\$

I have a piece of code that takes a dictionary as input and validates it based on the following rules:

  1. The dictionary has a key named items and its value must be a list
  2. Now, the list from point 1 must be a nested list with the length of each list should be 2
  3. The first element ([0]) of every nested list must be of type int and the second element ([1]) must be float

The input to validate:

json_to_test = {
    "items": [
        [
            1577837700,
            63.2
        ],
        [
            1577838600,
            61
        ],
        [
            1577839500,
            58.9
        ],
        [
            1577840400,
            57
        ],
        [
            1577841300,
            55.3
        ]
    ]
}

My code

def get_element(lst, num):
    return [item[num] for item in lst]

json_input = json_to_test['items']
if any(isinstance(i, list) for i in json_input):
    if all(len(i) == 2 for i in json_input):
        if all(isinstance(j, int) for j in get_element(json_input, 0)) and all(isinstance(j, float) for j in get_element(json_input, 1)):
            print('Data ready to be saved to DB')
            return json_input
        else:
            abort(400, 'Check data type')
    else:
        abort(400, 'Nested list but not lenghth 2')
else:
    abort(400, 'Not list')

The abort in the else statements is being imported from flask: https://flask.palletsprojects.com/en/1.1.x/api/#flask.abort

Is there a way the above code of mine be more efficient or further improved?

\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

Keyerror

Did you test this code with some invalid input?

The code will fail with a keyError if the items key does not exist, since your code just assumes it exists. You could do a try-except with the json_to_test['items'] so that you can handle the keyError.

Unspecificness

if any(isinstance(i, list) for i in json_input)

You're testing that json_input contains at least one element that is a list, but that could be something elese than json_to_test['items'] which you clearly said has to be a list. Why aren't you testing that particular object?

Nested code

Your code is triple-nested which makes it hard to read and to maintain.

It is better if you check each error condition separately without nesting and return only if they all pass. That also makes it much easier to add or remove another validation check without re-indenting the whole code and figuring out where it goes.

Something like this:

if fail_condition_1:
    abort("error_message_1")

if fail_condition_2:
    abort("error_message_2")

if fail_condition_3:
    abort("error_message_3")

print('Data ready to be saved to DB')
return json_input
```
\$\endgroup\$
1
  • \$\begingroup\$ Hey, thanks for your inputs. The did want to improve the nested code because, as pointed out by you, is hard to read and maintain. Regarding the keyerror and Unspecificness, I shall improve them for sure. Right now the code is being fed the correct information, that's why currently I didn't write specific tests for the above 2 cases. \$\endgroup\$ Commented Sep 22, 2020 at 11:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.