How to reduce number if-statements using dict?
Asked Answered
T

2

6

I have the following code with multiple cases:

def _extract_property_value(selector: Selector) -> str:
    raw_value = selector.xpath("span[2]")

    default_value = raw_value.xpath("./text()").get().strip()
    value_with_a = ', '.join([value.strip() for value in raw_value.xpath("./a /text()").getall()])
    value_with_div_and_a = ', '.join([value.strip() for value in raw_value.xpath("./div /a /text()").getall()])

    if value_with_a:
        return value_with_a
    elif value_with_div_and_a:
        return value_with_div_and_a
    elif default_value:
        return default_value

I want to get rid of if-statements and simplify this code as much as it is possible. I am not good Pyton dev. I know there is pattern "strategy" and below I was trying to implement that:

def _extract_property_value(selector: Selector) -> str:
    raw_value = selector.xpath("span[2]")
    values_dict = {
        'default value': raw_value.xpath("./text()").get().strip(),
        'value with a': ', '.join([value.strip() for value in raw_value.xpath("./a /text()").getall()]),
        'value with div and a': ', '.join([value.strip() for value in raw_value.xpath("./div /a /text()").getall()])
    }
    return [item for item in values_dict.values() if item != ''][0]

but... Now when I think of this - that was bad idea to use strategy there. I am not sure. Can someone help me to simplify that code? Or those if-statements are just necessary.

Teeming answered 4/4, 2022 at 10:42 Comment(2)
Welcome to Stack Overflow. You appear to have gotten confused, yes. A dict can be used to replace an if/elif/else structure that is used for dispatch - i.e., depending on multiple possibilities for one value, choose a course of action. But you want to use the structure for fallback - depending on whether one course of action was productive, possibly try the next etc. For this, you want to iterate over a sequence (such as a list) of strategies.Circosta
But more importantly: a Strategy should be something callable. Then you can look it up and call it according to the overall approach.Circosta
S
5

We can reduce the number of if statements but without the aid of a dictionary.

The code in question unconditionally assigns values to 3 variables. Having done so, those variables are examined to determine which, if any, is to be returned to the caller. However, there are no dependencies between those variables. Therefore they could be processed in order of priority and, if an appropriate value is acquired, then it could be returned immediately thereby making the process considerably more efficient.

def _extract_property_value(selector):
    def f1(rv):
        return rv.xpath("./text()").get().strip()
    def f2(rv):
        return ', '.join([value.strip() for value in rv.xpath("./a /text()").getall()])
    def f3(rv):
        return ', '.join([value.strip() for value in rv.xpath("./div /a /text()").getall()])
    raw_value = selector.xpath("span[2]")
    for func in [f2, f3, f1]: # note the priority order - default last
        if (v := func(raw_value)):
            return v

The revised function will implicitly return None if suitable values are not found. In this respect it is no different to the OP's original code

Sienese answered 4/4, 2022 at 11:13 Comment(0)
F
6

While you can certainly try using a Strategy pattern, there is a much simplier way to do this. Instead of the if/else chain you can simply do:

return value_with_a or value_with_div_and_a or default_value
Farmhouse answered 4/4, 2022 at 10:55 Comment(0)
S
5

We can reduce the number of if statements but without the aid of a dictionary.

The code in question unconditionally assigns values to 3 variables. Having done so, those variables are examined to determine which, if any, is to be returned to the caller. However, there are no dependencies between those variables. Therefore they could be processed in order of priority and, if an appropriate value is acquired, then it could be returned immediately thereby making the process considerably more efficient.

def _extract_property_value(selector):
    def f1(rv):
        return rv.xpath("./text()").get().strip()
    def f2(rv):
        return ', '.join([value.strip() for value in rv.xpath("./a /text()").getall()])
    def f3(rv):
        return ', '.join([value.strip() for value in rv.xpath("./div /a /text()").getall()])
    raw_value = selector.xpath("span[2]")
    for func in [f2, f3, f1]: # note the priority order - default last
        if (v := func(raw_value)):
            return v

The revised function will implicitly return None if suitable values are not found. In this respect it is no different to the OP's original code

Sienese answered 4/4, 2022 at 11:13 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.