Velocity Reviews

Velocity Reviews (http://www.velocityreviews.com/forums/index.php)
-   Python (http://www.velocityreviews.com/forums/f43-python.html)
-   -   Confused compare function :) (http://www.velocityreviews.com/forums/t955174-confused-compare-function.html)

Anatoli Hristov 12-05-2012 10:50 PM

Confused compare function :)
 
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 12-06-2012 12:31 AM

Re: Confused compare function :)
 
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,
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? :-P

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!



--
Steven

Anatoli Hristov 12-06-2012 09:36 AM

Re: Confused compare function :)
 
On Thu, Dec 6, 2012 at 1:31 AM, Steven D'Aprano
<steve+comp.lang.python@pearwood.info> wrote:
> 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,
> 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? :-P
>
> 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!
>
>
>
> --
> Steven
> --
> http://mail.python.org/mailman/listinfo/python-list


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


All times are GMT. The time now is 03:59 PM.

Powered by vBulletin®. Copyright ©2000 - 2014, vBulletin Solutions, Inc.
SEO by vBSEO ©2010, Crawlability, Inc.