Unnecessary code/code improvement

Create issue
Issue #1423 open
Cody Goodman created an issue

{{{ #!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.


Comments (2)

  1. Hynek Cernoch
    • changed status to open


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


  2. Log in to comment