- changed status to resolved
Unnecessary code/code improvement
{{{
!python
if pks:
- pks = [pk for pk in pks if _int_or_long(pk)]
-
pks = [pk for pk in pks if isinstance(pk, (int, long))] productdict = Product.objects.in_bulk(pks) products = [] for pk in pks: @@ -72,14 +72,3 @@ def highest_rated(count=0, site=None): products = []
return products
- -def _int_or_long(v):
- try:
- v = int(v)
- except ValueError:
- try:
- v = long(v)
- except ValueError:
- return False
- return True }}}
Sorry, I'm new to all of this. This is an improvement for satchmo_ext.productratings.queries. There is no need for a custom _int_or_long function, when the isinstance function is built into python.
Cody
Comments (2)
-
-
- changed status to open
Cody,
did you tried it? I think it can't work.
All "pk" used in
isinstance(pk, (int, long))
are strings and not integers or long. This would not work.
Maybere.match(r'\d+$', pk)
is a similar replacement.However you are righ that this module can be very much simplified, by more than one half, IMO.
I think that the original author added filtering because he could have a bad value in the cache during development from a previous bad code. It can be removed completely now.
The difference between int and long is not important here because e.g.
int(3) == long(3)
with all logical consequences for dictionaries etc. (I am sure in Python 2.5 and newer). It is sufficient to convert the value to int and not to take care too much that the result can be the long eventually. Also the testing that a key is in dict and also try/except is a duplicit code.Hynek
- Log in to comment
Slight code optimization for product ratings. Closes #1423
→ 9bcaeae31a1b