Confused compare function :)

[Anatoli Hristov]

Hi all,

I'm confused again with a compare update function. The problem is that
my function does not work at all and I don't get it where it comes
from.

in my DB I have total of 754 products. when I run the function is says:
Total updated: 754
Total not found with in the distributor: 747
I just don't get it, can you find my mistake ?

Thanks in advance

def Change_price():
total = 0
tnf = 0
for row in DB: # DB is mySQL DB, logically I get out 1 SKU and I
compare it with next loop
isku = row["sku"]
isku = isku.lower()
iprice = row["price"]
iprice = int(iprice)
found = 0
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 = 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()
stock = int(x[7])
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
print dsku, x[6], dprice, newprice
Update_SQL(newprice, isku)# goes to the SQL Update
print isku, newprice
if isku == dsku:# Just a check to see if it works
print "Found %s" %dsku
found = 1
else:
found = 0
except IndexError:
pass
except ValueError:
pass
except TypeError:
pass
except IndexError:
pass
if found == 1:
print "%s This is match" % isku
if found == 0:
print "%s Not found" % isku
tnf = tnf +1
total = total +1
print "Total updated: %s" % total
print"Total not found with in the distributor: %s" % tnf

 

[Steven D'Aprano]

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,
not from the database. Another misleading name, leading to confusion.

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
updates the 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
print "not found"
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:
print "%s Not found" % isku
tnf = tnf +1
total = total +1

Better to write this as:

if found:
print "%s This is match" % isku
else:
print "%s Not found" % isku
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

print"Total not found with in the distributor: %s" % tnf

Ah-ha! It means "Total Not Found"! I shouldn't have to read all the way
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!

 

[Anatoli Hristov]

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,
not from the database. Another misleading name, leading to confusion.

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
updates the 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
print "not found"
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:
print "%s Not found" % isku
tnf = tnf +1
total = total +1

Better to write this as:

if found:
print "%s This is match" % isku
else:
print "%s Not found" % isku
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

print"Total not found with in the distributor: %s" % tnf

Ah-ha! It means "Total Not Found"! I shouldn't have to read all the way
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!

Thank you Steven for your help. Now I renamed all the variables and it
seems working I didn't found the mistake, but it seems working Here
is the new code:

def Change_price(): # Changes the price in the DB if the price in the
CSV is changed
TotalUpdated = 0 # Counter for total updated
TotalNotFound = 0 # Counter for total not found
for row in PRODUCTSDB:
db_sku = row["sku"].lower()
db_price = int(row["price"])
found = False
try:
for x in pricelist:
try:
csv_price = x[6]
csv_price = csv_price.replace(",",".")
csv_price = float(csv_price)
csv_new_price = round(csv_price)*1.10
csv_sku = x[4].lower()
csv_stock = int(x[7]) # I used this as normally I
used stock in the condition
if len(db_sku) != 0 and db_sku == csv_sku and
csv_new_price < db_price and csv_stock > 0:
print db_sku, csv_price, db_price, csv_new_price
Update_SQL(csv_new_price, db_sku)
found = True
TotalUpdated += 1
else:
found = False

except IndexError: # I have a lot of index error in
the CSV (empty fields) and the loop gives "index error" I don't care
about them
pass
except ValueError:
pass
except TypeError:
pass
except IndexError:
pass
if found:
TotalNotFound += 1
print "%s This is match" % db_sku
else:
print "%s Not found" % db_sku
TotalNotFound += 1

print "Total updated: %s" % TotalUpdated
print"Total not found with in the distributor: %s" % TotalNotFound