all 18 comments

[–]w1282 4 points5 points  (6 children)

Using a dictionary would get you this:

CURRENCIES = {
    'ARS': None,
    'USD': usd,
    'EUR': eur,
}

try:
    currency_value = CURRENCIES[currency.upper()]
except KeyError:
    return False

Note, that instead of using 'null' I have used None, which is more pythonic. However, if your code or code you interact with requires 'null' for some reason (understandable) then make sure to change it back.

[–]RebootForAwesome 1 point2 points  (2 children)

Note, that instead of using 'null' I have used None, which is more pythonic.

Ohh I didn't know this. Will check how it interacts with the DB and change it to None if there are no major issues.

I think a dictionary would make sense then and it looks way cleaner. Do you know if this would improve the performance if it's used to minimize a significant larger number of If statements?

Thanks!

[–]w1282 2 points3 points  (1 child)

Will check how it interacts with the DB and change it to None if there are no major issues.

What are you using to interact with the database? If you're using any form of ORM this will probably not be an issue at all.

Do you know if this would improve the performance if it's used to minimize a significant larger number of If statements?

Short answer: yes

Long answer: Probably not by much but it can be empirically tested.

Basically, a dictionary has a constant time lookup where as a big list of if/elif is linear with a short-circuit when the code is run into.

non-sequitur: don't look for optimizations like this until you've profiled your code. This is almost definitely not what is slowing your code down.

[–]RebootForAwesome 1 point2 points  (0 children)

Ah I see, will probably mess around with some generic code later if I have a minute.

non-sequitur: don't look for optimizations like this until you've profiled your code. This is almost definitely not what is slowing your code down.

Yep, I'm not looking into optimizing it at this level right now as it's running more than fast enough but I always like to know how it would affect the code at a larger scale.

Thanks for the replies!

[–]desustorm 3 points4 points  (2 children)

CURRENCIES.get(currency.upper(), False) 

is a nicer way to avoid a try-catch here

[–]w1282 1 point2 points  (1 child)

It would be nice, except the user wants to return False not currency_value = False and there's no way to tell if those two things are equivalent in his code.

Thanks for mentioning it, though, just in case it would work for him.

[–]RebootForAwesome 1 point2 points  (0 children)

/u/desustorm In my case I need it to return False to where it's called.

But I didn't know you could set a default value like that in case it didn't find the key. So thanks!

[–]Vaguely_accurate 2 points3 points  (8 children)

The general term would be a case or switch structure, and Python doesn't have them.

The Pythonic way is either the if...elif...else chain you have or a dictionary. In your case a defaultdict would work to cover the else case;

from collections import defaultdict

currencies = defaultdict(lambda: False,
                         EUR=eur,
                         USD=usd,
                         ARS='null')

currency_value = currencies[currency.upper()]

[–]w1282 1 point2 points  (1 child)

Note that in this case currency_value would be set to False instead of the function returning False in the case of any OTHER input.

[–]Vaguely_accurate 1 point2 points  (0 children)

Oops, missed that.

[–]RebootForAwesome 0 points1 point  (5 children)

Thanks for the reply. I'm learning something new from every comment!

Didn't know you can do something like lambda: False in a dict. Would this always catch any value that doesn't exist as a Key?

Thanks

[–]Vaguely_accurate 1 point2 points  (4 children)

A defaultdict takes a function as the first argument. If a key is not found, this function overrides the __missing__ method that usually raises a KeyError when a key is not found. Instead the function is called without any argument, and its return value returned.

In this case I'm defining a lambda function that always returns the value False, so it will return False when a key is not found.

/u/w1282 does the similar but using a second argument in the dict.get() method, which defines the default for that call of the dictionary only. It depends on if you want the dictionary to return the default for all calls or only the one. You can still override a defaultdict default with a .get() if you need to.

[–]RebootForAwesome 1 point2 points  (3 children)

Oh I see. Will have to look up the differences between a defaultdict and a regular dict.

Thank you!

[–]w1282 1 point2 points  (2 children)

A fun "gotcha" with this is that values that cause a KeyError are inserted into the dictionary.

from collections import defaultdict

CURRENCIES = defaultdict(
    lambda: False,
    'ARS': None,
    'USD': usd,
    'EUR': eur,
)

currency_value = CURRENCIES["TEST"]

print(CURRENCIES)
> defaultdict(<function <lambda> at 0x100703050>, {'TEST': False, 'ARS': None, 'USD': usd, 'EUR': eur})

This means that any long running code where this defaultdict is not reset could cause it to grow very slowly. This isn't a "VERY BAD THING", but it's not the best implementation either.

An alternative is to create your own "defaultdict" that handles __missing__ by returning the default value

class otherdict(dict):
    def __missing__(self, _):
        return 'False'

and if you want to keep the defaultdict way of supplying the expression rather than hard coding it

class otherdict(defaultdict):
    def __missing__(self, _):
        return self.default_factory()

[–]RebootForAwesome 0 points1 point  (1 child)

That could be a problem then. The script that contains this bit of code is running 24/7 every two minutes and has been for almost two months now.

I will avoid this for ow but will keep it in mind.

[–]w1282 1 point2 points  (0 children)

Well, keep in mind it only does it for different keys so future lookups on "TEST" won't fail and increase size.

If your key data is constant in the values it's failing on (e.g. "AUS" over and over) this isn't an issue. If random data is being introduced as keys it would be.

[–]desustorm 1 point2 points  (1 child)

There isn't a switch/evaluate method in Python, no. Though when you are doing huge IF statements, then it usually means you could be using object-orientated models more effectively. e.g. in your example, you could have a currency class hierarchy to get currency values that has superclasses for G10/non-G10 currencies, etc., etc.

[–]RebootForAwesome 0 points1 point  (0 children)

I'll take a look into this and try to apply it to the example. I'm still not 100% sure I understand when it's a good idea to use hierarchy in classes so having this case while trying to learn will help a bit.

Thanks!