# Confused compare function :)

Steven D'Aprano steve+comp.lang.python at pearwood.info
Thu Dec 6 01:31:48 CET 2012

```On Wed, 05 Dec 2012 23:50:49 +0100, Anatoli Hristov wrote:

> def Change_price():

Misleading function name. What price does it change?

>     total = 0
>     tnf = 0

"tnf"? Does that mean something?

>     for row in DB:  # DB is mySQL DB, logically I get out
>                     # 1 SKU and I compare it with next loop

Use of global variables, yuck. What happens if some day you need two
databases at the same time?

>         isku = row["sku"]
>         isku = isku.lower()

Hungarian Notation? This is better written as:

sku = row["sku"].lower()

>         iprice = row["price"]
>         iprice = int(iprice)

And likewise price = int(row["price"]). Or better still, "oldprice", or
"price_in_database", or something that actually describes what it is.

>         found = 0

found = False

>         try:
>             for x in PRICELIST:  # here is my next loop in a CSV file
>                                  # which is allready in a list PRICELIST
>                 try:
>                     dprice = x[6]

"dprice"? D-for-database price? But this is the price from the CSV file,

>                     dprice = dprice.replace(",",".")
>                     # As in the PRICELIST the prices are with
>                     # commas I replace the comma as python request it
>                     dprice = float(dprice)
>                     newprice = round(dprice)*1.10
>                     dsku = x[4]
>                     dsku = dsku.lower()

And again, what's "dsku" mean? Database-SKU? But it's the CSV SKU.

>                     stock = int(x[7])

I don't believe that this is used at all. Get rid of it.

>                     if isku == dsku and newprice < int(iprice):
>                     # If found the coresponded SKU and the price is
>                     # higher than the one in the CSV I update the price

I think your logic is wrong here. You aren't comparing the price in the
CSV here at all. You compare two prices, neither of which is the price in
the CSV file:

newprice = round(price in CSV) * 1.10
iprice = price from the database

(which is already an int, no need to call int *again* -- if you're going
to use Hungarian Notation, pay attention to it!)

So you have THREE prices, not two, and it isn't clear which ones you are
*supposed* to compare. Either the code is wrong, or the comment is wrong,
or possibly both.

iprice = price from the database
dprice = price from the CSV
newprice = calculated new price

I'm going to GUESS that you actually want to compare the new price with
the price in the database.

if newprice > iprice:  # horrible name! no wonder you are confused
# Update the database with the new price

>                         print dsku, x[6], dprice, newprice
>                         Update_SQL(newprice, isku)
>                         # goes to the SQL Update

Really? Gosh, without the comment, how would anyone know that Update_SQL

Seriously, the comment is redundant. Get rid of it.

>                         print isku, newprice
>                         if isku == dsku:
>                         # Just a check to see if it works
>                             print "Found %s" %dsku
>                             found = 1
>                         else:
>                             found = 0

found = True or False is better.

But this code cannot do anything but print Found, since above you already
tested that isku == dsku. So this check is pointless.

The reason is, your code does this:

if isku == dsku and (something else):
# Inside this block, isku MUST equal dsku
blah blah blah
if isku == dsku:
print "Found"
found = 1
else:
# But this cannot possibly happen
found = 0

>                 except IndexError:
>                     pass
>                 except ValueError:
>                     pass
>                 except TypeError:
>                     pass

Why are you hiding errors? You should not hide errors unnecessarily, that
means there are bugs in either the CSV or your code, you should fix the
bugs.

However, if you really must, then you can replace all of those with:

except (IndexError, ValueError, TypeError):
pass

>         except IndexError:
>             pass

And hiding more errors?

>         if found == 1:
>             print "%s This is match" % isku
>         if found == 0:
>             tnf = tnf +1
>         total = total +1

Better to write this as:

if found:
print "%s This is match" % isku
else:
tnf = tnf + 1  # What does this mean?
total += 1

>     print "Total updated: %s" % total

That's wrong. total is *not* the number updated. It is the total, updated
or not updated. This should say:

print "Total records inspected: %s" % total

to the end of the code to discover this. Instead of "tnf", you should
count the total number of SKUs, and count how many times you call
Update_SQL. Then you can report:

Total records inspected: 754
Total records updated: 392

(or whatever the values are).

Simplify and clean up your code, and then it will be easier to find and
fix the problems in it. Good luck!

--
Steven

```