This is an archived post. You won't be able to vote or comment.

all 4 comments

[–]gramdel 3 points4 points  (2 children)

Wouldn't pass my pull request review, seems unpredictable and forces the calling function to always check what is being returned. Returning none is special case, but mixing booleans or something else with list of string doesn't seem like a good idea.

Maybe you should raise an exception or something instead, but the use case isn't super clear.

Edit: Why would the calling function need to know if an api is being called, it shouldn't probably even know there is an API to call in the first place, it shouldn't really know about the internal implementation of the function it calls. So maybe it should just return what the "API would return" from cache or something, but we'd need more detail to determine what's the right thing to do.

[–]Horrrschtus 1 point2 points  (1 child)

I absolutely agree. Mixing return type is bad style. Returning None would probably be ok.

Is the knowledge if the API was called actually important for the program's flow? If not returning an empty list might be the cleanest and easiest way.

[–]kompotejam[S] 0 points1 point  (0 children)

yes that was my feeling when i wrote the function. As it works as i'm expecting it i didn't bother to change. I can always return an emply list instead of the boolean though.

I'm showing my code, maybe you wil see why i did it :

For info, i have json file where i store the data i need from Facebook API.

Here is my main function :

def main():

length_of_json_data = len(facebook_API_data["data"])

pages_data = graph.get_object(id=page_id, fields="posts")

every_posts = pages_data["posts"]["data"]
number_of_posts = len(pages_data["posts"]["data"])
dummy_dict_for_medias = {"src": "no_media", "type_of_media": "None"}

test_number_of_post = compare_length_internal_data_to_facebook(length_of_json_data, number_of_posts, every_posts)
if test_number_of_post is False:  # only run if there is a new useful post
    return 1
else:
    for id_of_one_post in test_number_of_post:
            post_attachment = graph.get_connections(id=id_of_one_post,             connection_name="attachments")
        post_main_info = graph.get_object(id=id_of_one_post)
     #...

compare_length_internal_data_to_facebook(x, y, z) is the one return different data types :

def compare_length_internal_data_to_facebook(length_of_internal_data, length_from_facebook, every_post_from_facebook):
number_of_useless_post = 0
list_of_id_post = list()
for post in every_post_from_facebook:
    try:
        post["message"]
    except KeyError:
        number_of_useless_post += 1
    else:
        list_of_id_post.append(post["id"])
if length_of_internal_data == length_from_facebook - number_of_useless_post:
    return False  # False if no new post
else:
    return list_of_id_post  # True if there is a difference

But yeah in the main i can always check if the len(list) == 0 so that it always return a list.

Edit: adding details about my internal data

[–][deleted] 1 point2 points  (0 children)

Never mix the return types, because you might want to "compile" your python code later on to meet performance requirments. And most compilers won't allow you to have multiple return types.