jmichaelward / py-bgg

A Python playground.
1 stars 0 forks source link

Pythonic for 0 == len(foo) #1

Open genghis opened 4 years ago

genghis commented 4 years ago

Just as a note, the pythonic way to check if a collection is length 0 is to use

if not foo:

instead of

if 0 == len(foo):

genghis commented 4 years ago

Concrete example of another one--

Line 32 of src/api/users.py.

if userdata['user']['@id'] == ''

Pythonically is--

if not userdata['user']['@id']:

genghis commented 4 years ago

Pythonistas love checking for the truthy or falsy existence of shit.

jmichaelward commented 4 years ago

I'll give this a try.

I think an important next step for this app is to start writing tests against some of these methods, as that'll be a great way to validate whether I'm consistent getting the expected return value. I'm still learning the ropes of the language and what's truthy or falsy. In PHP, I'd definitely do something like:

$some_array = get_some_array();

if ( ! $some_array ) {
    // bail
}
jmichaelward commented 4 years ago

I updated line 32 of the referenced file, and it looks like my app refused to try to create the user without throwing an error, as I'd expect. I'll check for other instances of that type to see if I can use the not comparison instead. Thanks!

jmichaelward commented 4 years ago

Hey! This, in combination with #2, helped me find a bug. See: https://github.com/jmichaelward/py-bgg/commit/5f0614196168ad31b9e42dcafc180dced7399b11 https://github.com/jmichaelward/py-bgg/commit/7460cafcbe40e628992834247d6e20f68b88612b

The api_handle_add method is supposed to return an existing user from the database if it already exists, but the check on line 23 was failing after I changed it because the length of the dictionary was 2, not one, because a dictionary's length is based on how many keys it has. I think I had changed the logic from before, because before I was returning a list of dictionaries.

Anyway, after changing that line in the first commit, I came to learn about the changed behavior because the app would fail when I tried to add an existing username. Removing the index lookups in the condition fixed the problem.

This re-asserts the need for tests, most importantly, by I think your point about learning the truthy/falsy checks are well-founded. Thanks again for leaving this feedback.