On Thu, Dec 6, 2012 at 1:31 AM, Steven D'Aprano
<steve+comp.lang.pyt...@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
--
http://mail.python.org/mailman/listinfo/python-list
