Confused compare function :)

Discussion in 'Python' started by Anatoli Hristov, Dec 5, 2012.

  1. 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
    Anatoli Hristov, Dec 5, 2012
    #1
    1. Advertising

  2. 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
    Steven D'Aprano, Dec 6, 2012
    #2
    1. Advertising

  3. On Thu, Dec 6, 2012 at 1:31 AM, Steven D'Aprano
    <> 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
    Anatoli Hristov, Dec 6, 2012
    #3
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Sullivan WxPyQtKinter
    Replies:
    5
    Views:
    420
    Rene Pijlman
    Mar 7, 2006
  2. Xiaoshen Li

    confused with function declarations

    Xiaoshen Li, Dec 21, 2005, in forum: C Programming
    Replies:
    10
    Views:
    520
    Flash Gordon
    Dec 22, 2005
  3. Lee Fleming
    Replies:
    49
    Views:
    796
    Steve Holden
    Aug 14, 2007
  4. xkenneth
    Replies:
    7
    Views:
    415
  5. Bruno Dupuis

    Re: Confused compare function :)

    Bruno Dupuis, Dec 6, 2012, in forum: Python
    Replies:
    40
    Views:
    532
    Steven D'Aprano
    Dec 9, 2012
Loading...

Share This Page