Khakis7
Khakis7

Reputation: 433

Cleaner solution to handling variable setting than multiple if statements

I have a set of if statements that set a variable based on the type of operation I'm attempting to do

if reverse_relationship and not get_all_links:
    traversal_function = 'reverse'
    annotation_function = 'forward'
elif reverse_relationship and get_all_links:
    traversal_function = 'all_reverse'
    annotation_function = 'all_forward'
    item_id_query = 'reverse'
elif get_all_links:
    traversal_function = 'all_forward'
    annotation_function = 'all_reverse'
    item_id_query = 'forward'
else:
    traversal_function = 'forward'
    annotation_function = 'reverse'

But I feel like there must be a much easier way of doing this, as the above can be pretty tough to read.

Upvotes: 1

Views: 56

Answers (2)

CryptoFool
CryptoFool

Reputation: 23139

You can do this more concisely using a dictionary where the keys are tuples containing the boolean inputs to your if statements, and the values are the resulting three strings:

map = {
    (True, False): ('reverse', 'forward', None),
    (True, True): ('all_reverse', 'all_forward', 'reverse'),
    (False, True): ('all_forward', 'all_reverse', 'forward'),
    (False, False): ('forward', 'reverse', None),
}

traversal_function, annotation_function, item_id_query = map[(reverse_relationship, get_all_links)]

@konserw's second answer is another way to go if you want to make use of the fact that the resulting strings are built up in a logical way based on the inputs. Be aware that the solution they present results in item_query_id having a non-None value in all cases though, which doesn't match your code. To get the same results as your solution, making only a change to see that get_all_links is always defined, but is None for the cases where you don't set it in your code, you could change their answer to this:

item_id_query = None
traversal_function = 'reverse' if reverse_relationship else 'forward'
annotation_function = 'forward' if reverse_relationship else 'reverse'
if get_all_links:
    item_id_query = traversal_function
    traversal_function = f'all_{traversal_function}'
    annotation_function = f'all_{annotation_function}'

To have this code act exactly the same as your if statements, just remove the first line of this code. I don't recommend that, as I think you'd want item_id_query to be defined in all cases.

Upvotes: 1

konserw
konserw

Reputation: 554

Something like:

if reverse_relationship:
    if get_all_links:
        traversal_function = 'all_reverse'
        annotation_function = 'all_forward'
        item_id_query = 'reverse'
    else:
        traversal_function = 'reverse'
        annotation_function = 'forward'
else:
    if get_all_links:
        traversal_function = 'all_forward'
        annotation_function = 'all_reverse'
        item_id_query = 'forward'
    else:
        traversal_function = 'forward'
        annotation_function = 'reverse'

Also notice that you may have item_id_query undefined

EDIT: more compact version

item_id_query = 'reverse' if reverse_relationship else 'forward'
traversal_function = item_id_query
annotation_function = 'forward' if reverse_relationship else 'reverse'
if get_all_links:
    traversal_function = f'all_{traversal_function}'
    annotation_function = f'all_{annotation_function}'

Upvotes: 0

Related Questions