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.
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
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