Python update SQLite DB Values

The name of the pictureThe name of the pictureThe name of the pictureClash Royale CLAN TAG#URR8PPP












5












$begingroup$


At the moment I try to get used to SQLite and Python. I started with insert, select and so on. But now i want to get a bit further.
Now I have a DB with 200 rows for testing and in want to update two values of each row in this case called "Test2" and "Test3". My problem is that those two values should be the value that they have +200 and +400 so i have to make an select before updating them (at least with my knowledge).



I added a simple time measurement to my code to see if i will be able to make my code more efficient and faster!



At the moment it takes 0.7344 seconds for me to edit 200 rows of data. Which is pretty slow! Especially if i think to do it for bigger DB's.



Things I want to achieve:



  1. Is it really necessary to make the select or could I use the present Data in any other way?

  2. Make things faster! Is it possible to edit multiple rows at the same time? Like parallelized tasks?

Any other tips are welcome, as i try to improve myself!



Here is my code so far:



import sqlite3
import time

def database_test():
# Open Database
conn = sqlite3.connect('SQLite_Test.db')
c = conn.cursor()

i = 0

for i in range(200):
c.execute('SELECT Test2, Test3 FROM Test WHERE Test1 = ?', (i,))

DB_Values =
DB_Values = c.fetchone()

Value1 = DB_Values[0]+200
Value2 = DB_Values[1]+400

c.execute('''UPDATE Test SET Test2 = ?, Test3 = ? WHERE Test1= ?''', (Value1, Value2, i))
i += 1

# Save (commit) the changes
conn.commit()


start_time = time.time()
database_test()
print("--- %s seconds ---" % round((time.time() - start_time),4))









share|improve this question









$endgroup$
















    5












    $begingroup$


    At the moment I try to get used to SQLite and Python. I started with insert, select and so on. But now i want to get a bit further.
    Now I have a DB with 200 rows for testing and in want to update two values of each row in this case called "Test2" and "Test3". My problem is that those two values should be the value that they have +200 and +400 so i have to make an select before updating them (at least with my knowledge).



    I added a simple time measurement to my code to see if i will be able to make my code more efficient and faster!



    At the moment it takes 0.7344 seconds for me to edit 200 rows of data. Which is pretty slow! Especially if i think to do it for bigger DB's.



    Things I want to achieve:



    1. Is it really necessary to make the select or could I use the present Data in any other way?

    2. Make things faster! Is it possible to edit multiple rows at the same time? Like parallelized tasks?

    Any other tips are welcome, as i try to improve myself!



    Here is my code so far:



    import sqlite3
    import time

    def database_test():
    # Open Database
    conn = sqlite3.connect('SQLite_Test.db')
    c = conn.cursor()

    i = 0

    for i in range(200):
    c.execute('SELECT Test2, Test3 FROM Test WHERE Test1 = ?', (i,))

    DB_Values =
    DB_Values = c.fetchone()

    Value1 = DB_Values[0]+200
    Value2 = DB_Values[1]+400

    c.execute('''UPDATE Test SET Test2 = ?, Test3 = ? WHERE Test1= ?''', (Value1, Value2, i))
    i += 1

    # Save (commit) the changes
    conn.commit()


    start_time = time.time()
    database_test()
    print("--- %s seconds ---" % round((time.time() - start_time),4))









    share|improve this question









    $endgroup$














      5












      5








      5





      $begingroup$


      At the moment I try to get used to SQLite and Python. I started with insert, select and so on. But now i want to get a bit further.
      Now I have a DB with 200 rows for testing and in want to update two values of each row in this case called "Test2" and "Test3". My problem is that those two values should be the value that they have +200 and +400 so i have to make an select before updating them (at least with my knowledge).



      I added a simple time measurement to my code to see if i will be able to make my code more efficient and faster!



      At the moment it takes 0.7344 seconds for me to edit 200 rows of data. Which is pretty slow! Especially if i think to do it for bigger DB's.



      Things I want to achieve:



      1. Is it really necessary to make the select or could I use the present Data in any other way?

      2. Make things faster! Is it possible to edit multiple rows at the same time? Like parallelized tasks?

      Any other tips are welcome, as i try to improve myself!



      Here is my code so far:



      import sqlite3
      import time

      def database_test():
      # Open Database
      conn = sqlite3.connect('SQLite_Test.db')
      c = conn.cursor()

      i = 0

      for i in range(200):
      c.execute('SELECT Test2, Test3 FROM Test WHERE Test1 = ?', (i,))

      DB_Values =
      DB_Values = c.fetchone()

      Value1 = DB_Values[0]+200
      Value2 = DB_Values[1]+400

      c.execute('''UPDATE Test SET Test2 = ?, Test3 = ? WHERE Test1= ?''', (Value1, Value2, i))
      i += 1

      # Save (commit) the changes
      conn.commit()


      start_time = time.time()
      database_test()
      print("--- %s seconds ---" % round((time.time() - start_time),4))









      share|improve this question









      $endgroup$




      At the moment I try to get used to SQLite and Python. I started with insert, select and so on. But now i want to get a bit further.
      Now I have a DB with 200 rows for testing and in want to update two values of each row in this case called "Test2" and "Test3". My problem is that those two values should be the value that they have +200 and +400 so i have to make an select before updating them (at least with my knowledge).



      I added a simple time measurement to my code to see if i will be able to make my code more efficient and faster!



      At the moment it takes 0.7344 seconds for me to edit 200 rows of data. Which is pretty slow! Especially if i think to do it for bigger DB's.



      Things I want to achieve:



      1. Is it really necessary to make the select or could I use the present Data in any other way?

      2. Make things faster! Is it possible to edit multiple rows at the same time? Like parallelized tasks?

      Any other tips are welcome, as i try to improve myself!



      Here is my code so far:



      import sqlite3
      import time

      def database_test():
      # Open Database
      conn = sqlite3.connect('SQLite_Test.db')
      c = conn.cursor()

      i = 0

      for i in range(200):
      c.execute('SELECT Test2, Test3 FROM Test WHERE Test1 = ?', (i,))

      DB_Values =
      DB_Values = c.fetchone()

      Value1 = DB_Values[0]+200
      Value2 = DB_Values[1]+400

      c.execute('''UPDATE Test SET Test2 = ?, Test3 = ? WHERE Test1= ?''', (Value1, Value2, i))
      i += 1

      # Save (commit) the changes
      conn.commit()


      start_time = time.time()
      database_test()
      print("--- %s seconds ---" % round((time.time() - start_time),4))






      python performance beginner sqlite






      share|improve this question













      share|improve this question











      share|improve this question




      share|improve this question










      asked Feb 12 at 7:26









      K-DoeK-Doe

      955




      955




















          2 Answers
          2






          active

          oldest

          votes


















          8












          $begingroup$

          SQL (and thus SQLite) support a variety of operators suitable (amongst others) in UPDATE statements.



          This means that you can simplify your whole loop into a single statement:



          query = '''
          UPDATE Test SET Test2 = (Test2 + ?), Test3 = (Test3 + ?)
          WHERE Test1 >= 0 AND Test1 < 200
          '''
          c.execute(query, (200, 400))



          Also you should take the habit to use perf_counter instead of time to measure elapsed time. Or use the dedicated timeit module.






          share|improve this answer









          $endgroup$




















            5












            $begingroup$

            Mathias’ answer is the correct way of solving your problem but since this is a code review site, let’s look at the rest of your code.



            First off, your code style is fundamentally tidy and clear, so that’s very good. Now for some detailed feedback:




             # Open Database



            Comments such as this one are at best unnecessary and at worst harmful: the comment provides no additional information compared to the code, and at worst they are misleading. Case in point, what happens if the file SQLite_Test.db doesn’t exist yet? Your comment gives no indication — it implies that the file must exist. But SQLite will actually happily create it for you if it doesn’t exist yet.



            Jessica Baker has written a very good article explaining how to write comments well. I strongly urge anyone to read it in its entirety.




             c = conn.cursor()



            Your code doesn’t need database cursors: Although the subsequent code uses c, it could use conn instead.




             i = 0



            This initialisation is redundant: range(…) does it for you.




             for i in range(200):



            Why 200? Avoid hard-coding “magic” numbers. If this is the size of your database table, don’t put the number in code, compute it from the database. If the number is arbitrary (for testing, say), assign it to a variable stating thus.




             DB_Values = 
            DB_Values = c.fetchone()



            The first line here sets DB_Values to an empty list; the second line immediately overrides that value. The first lines is therefore unnecessary.



            Apart from this, you should respect PEP 8 naming conventions. Lastly, db_values isn’t a very clear name: db_row might be more expressive.




             Value1 = DB_Values[0]+200
            Value2 = DB_Values[1]+400



            Same comment regarding names, and same comment regarding magic numbers.




             c.execute('''UPDATE Test SET Test2 = ?, Test3 = ? WHERE Test1= ?''', (Value1, Value2, i))
            i += 1



            You might think that it’s necessary to increment i to progress the loop but this isn’t the case. The increment of i fulfils no purpose, since it will be overridden by the for loop anyway.




             # Save (commit) the changes



            Same as above: this comment isn’t helpful, since it just repeats what the code says.




             conn.commit()



            If there was an exception in the preceding code, this commit will never be executed. You may have intended this; but chances are, you didn’t. It’s therefore best practice to avoid implicit resource cleanup (including database commits), and to use Python’s with statement instead.




            Taking this in account, here’s a rewritten version of your code:



            import sqlite3
            import time

            TEST_NUM_ROWS = 200
            TEST2_INC = 200
            TEST3_INC = 400

            def database_test():
            with sqlite3.connect('SQLite_Test.db') as conn:
            for i in range(TEST_NUM_ROWS):
            row = conn.execute(
            'SELECT Test2, Test3 FROM Test WHERE Test1 = ?',
            (i,)
            ).fetchone()
            new_row = (row[0] + TEST2_INC, row[1] + TEST3_INC)
            conn.execute(
            'UPDATE Test SET Test2 = ?, Test3 = ? WHERE Test1 = ?',
            new_row + (i,)
            )


            start_time = time.time()
            database_test()
            print("--- %s seconds ---" % round((time.time() - start_time), 4))





            share|improve this answer











            $endgroup$








            • 1




              $begingroup$
              I like this answer except how you use the context manager. There always seem to be some level of confusion about what the context manager on a DB connection is doing. To avoid people thinking that the connection will be closed after executing the with block, I usually separate the connect call from the with statement as shown in the docs and wrap the connect call in a with contextlib.closing(..) as conn if need be.
              $endgroup$
              – Mathias Ettinger
              Feb 12 at 16:30











            • $begingroup$
              @MathiasEttinger I understand your concern but separating the initialisation from the context manager doesn’t fix the issue: every competent Python programmer knows that the resulting code is in fact equivalent. Rather, this needs to be fixed at the API level in sqlite3; e.g. as with conn.transaction() as t: ….
              $endgroup$
              – Konrad Rudolph
              Feb 12 at 17:22











            • $begingroup$
              Well, that is an other argument to separate the with conn: call from the connect one (as it would be nearly useless to do with sqlite3.connect(...).transaction()) ;)
              $endgroup$
              – Mathias Ettinger
              Feb 12 at 18:24










            • $begingroup$
              @MathiasEttinger If that API existed I’d agree, yes.
              $endgroup$
              – Konrad Rudolph
              Feb 12 at 18:40










            • $begingroup$
              Thank you for your well written answere! I will learn alot from it.
              $endgroup$
              – K-Doe
              Feb 13 at 6:32










            Your Answer





            StackExchange.ifUsing("editor", function ()
            return StackExchange.using("mathjaxEditing", function ()
            StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix)
            StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
            );
            );
            , "mathjax-editing");

            StackExchange.ifUsing("editor", function ()
            StackExchange.using("externalEditor", function ()
            StackExchange.using("snippets", function ()
            StackExchange.snippets.init();
            );
            );
            , "code-snippets");

            StackExchange.ready(function()
            var channelOptions =
            tags: "".split(" "),
            id: "196"
            ;
            initTagRenderer("".split(" "), "".split(" "), channelOptions);

            StackExchange.using("externalEditor", function()
            // Have to fire editor after snippets, if snippets enabled
            if (StackExchange.settings.snippets.snippetsEnabled)
            StackExchange.using("snippets", function()
            createEditor();
            );

            else
            createEditor();

            );

            function createEditor()
            StackExchange.prepareEditor(
            heartbeatType: 'answer',
            autoActivateHeartbeat: false,
            convertImagesToLinks: false,
            noModals: true,
            showLowRepImageUploadWarning: true,
            reputationToPostImages: null,
            bindNavPrevention: true,
            postfix: "",
            imageUploader:
            brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
            contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
            allowUrls: true
            ,
            onDemand: true,
            discardSelector: ".discard-answer"
            ,immediatelyShowMarkdownHelp:true
            );



            );













            draft saved

            draft discarded


















            StackExchange.ready(
            function ()
            StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f213288%2fpython-update-sqlite-db-values%23new-answer', 'question_page');

            );

            Post as a guest















            Required, but never shown

























            2 Answers
            2






            active

            oldest

            votes








            2 Answers
            2






            active

            oldest

            votes









            active

            oldest

            votes






            active

            oldest

            votes









            8












            $begingroup$

            SQL (and thus SQLite) support a variety of operators suitable (amongst others) in UPDATE statements.



            This means that you can simplify your whole loop into a single statement:



            query = '''
            UPDATE Test SET Test2 = (Test2 + ?), Test3 = (Test3 + ?)
            WHERE Test1 >= 0 AND Test1 < 200
            '''
            c.execute(query, (200, 400))



            Also you should take the habit to use perf_counter instead of time to measure elapsed time. Or use the dedicated timeit module.






            share|improve this answer









            $endgroup$

















              8












              $begingroup$

              SQL (and thus SQLite) support a variety of operators suitable (amongst others) in UPDATE statements.



              This means that you can simplify your whole loop into a single statement:



              query = '''
              UPDATE Test SET Test2 = (Test2 + ?), Test3 = (Test3 + ?)
              WHERE Test1 >= 0 AND Test1 < 200
              '''
              c.execute(query, (200, 400))



              Also you should take the habit to use perf_counter instead of time to measure elapsed time. Or use the dedicated timeit module.






              share|improve this answer









              $endgroup$















                8












                8








                8





                $begingroup$

                SQL (and thus SQLite) support a variety of operators suitable (amongst others) in UPDATE statements.



                This means that you can simplify your whole loop into a single statement:



                query = '''
                UPDATE Test SET Test2 = (Test2 + ?), Test3 = (Test3 + ?)
                WHERE Test1 >= 0 AND Test1 < 200
                '''
                c.execute(query, (200, 400))



                Also you should take the habit to use perf_counter instead of time to measure elapsed time. Or use the dedicated timeit module.






                share|improve this answer









                $endgroup$



                SQL (and thus SQLite) support a variety of operators suitable (amongst others) in UPDATE statements.



                This means that you can simplify your whole loop into a single statement:



                query = '''
                UPDATE Test SET Test2 = (Test2 + ?), Test3 = (Test3 + ?)
                WHERE Test1 >= 0 AND Test1 < 200
                '''
                c.execute(query, (200, 400))



                Also you should take the habit to use perf_counter instead of time to measure elapsed time. Or use the dedicated timeit module.







                share|improve this answer












                share|improve this answer



                share|improve this answer










                answered Feb 12 at 9:20









                Mathias EttingerMathias Ettinger

                25.1k33184




                25.1k33184























                    5












                    $begingroup$

                    Mathias’ answer is the correct way of solving your problem but since this is a code review site, let’s look at the rest of your code.



                    First off, your code style is fundamentally tidy and clear, so that’s very good. Now for some detailed feedback:




                     # Open Database



                    Comments such as this one are at best unnecessary and at worst harmful: the comment provides no additional information compared to the code, and at worst they are misleading. Case in point, what happens if the file SQLite_Test.db doesn’t exist yet? Your comment gives no indication — it implies that the file must exist. But SQLite will actually happily create it for you if it doesn’t exist yet.



                    Jessica Baker has written a very good article explaining how to write comments well. I strongly urge anyone to read it in its entirety.




                     c = conn.cursor()



                    Your code doesn’t need database cursors: Although the subsequent code uses c, it could use conn instead.




                     i = 0



                    This initialisation is redundant: range(…) does it for you.




                     for i in range(200):



                    Why 200? Avoid hard-coding “magic” numbers. If this is the size of your database table, don’t put the number in code, compute it from the database. If the number is arbitrary (for testing, say), assign it to a variable stating thus.




                     DB_Values = 
                    DB_Values = c.fetchone()



                    The first line here sets DB_Values to an empty list; the second line immediately overrides that value. The first lines is therefore unnecessary.



                    Apart from this, you should respect PEP 8 naming conventions. Lastly, db_values isn’t a very clear name: db_row might be more expressive.




                     Value1 = DB_Values[0]+200
                    Value2 = DB_Values[1]+400



                    Same comment regarding names, and same comment regarding magic numbers.




                     c.execute('''UPDATE Test SET Test2 = ?, Test3 = ? WHERE Test1= ?''', (Value1, Value2, i))
                    i += 1



                    You might think that it’s necessary to increment i to progress the loop but this isn’t the case. The increment of i fulfils no purpose, since it will be overridden by the for loop anyway.




                     # Save (commit) the changes



                    Same as above: this comment isn’t helpful, since it just repeats what the code says.




                     conn.commit()



                    If there was an exception in the preceding code, this commit will never be executed. You may have intended this; but chances are, you didn’t. It’s therefore best practice to avoid implicit resource cleanup (including database commits), and to use Python’s with statement instead.




                    Taking this in account, here’s a rewritten version of your code:



                    import sqlite3
                    import time

                    TEST_NUM_ROWS = 200
                    TEST2_INC = 200
                    TEST3_INC = 400

                    def database_test():
                    with sqlite3.connect('SQLite_Test.db') as conn:
                    for i in range(TEST_NUM_ROWS):
                    row = conn.execute(
                    'SELECT Test2, Test3 FROM Test WHERE Test1 = ?',
                    (i,)
                    ).fetchone()
                    new_row = (row[0] + TEST2_INC, row[1] + TEST3_INC)
                    conn.execute(
                    'UPDATE Test SET Test2 = ?, Test3 = ? WHERE Test1 = ?',
                    new_row + (i,)
                    )


                    start_time = time.time()
                    database_test()
                    print("--- %s seconds ---" % round((time.time() - start_time), 4))





                    share|improve this answer











                    $endgroup$








                    • 1




                      $begingroup$
                      I like this answer except how you use the context manager. There always seem to be some level of confusion about what the context manager on a DB connection is doing. To avoid people thinking that the connection will be closed after executing the with block, I usually separate the connect call from the with statement as shown in the docs and wrap the connect call in a with contextlib.closing(..) as conn if need be.
                      $endgroup$
                      – Mathias Ettinger
                      Feb 12 at 16:30











                    • $begingroup$
                      @MathiasEttinger I understand your concern but separating the initialisation from the context manager doesn’t fix the issue: every competent Python programmer knows that the resulting code is in fact equivalent. Rather, this needs to be fixed at the API level in sqlite3; e.g. as with conn.transaction() as t: ….
                      $endgroup$
                      – Konrad Rudolph
                      Feb 12 at 17:22











                    • $begingroup$
                      Well, that is an other argument to separate the with conn: call from the connect one (as it would be nearly useless to do with sqlite3.connect(...).transaction()) ;)
                      $endgroup$
                      – Mathias Ettinger
                      Feb 12 at 18:24










                    • $begingroup$
                      @MathiasEttinger If that API existed I’d agree, yes.
                      $endgroup$
                      – Konrad Rudolph
                      Feb 12 at 18:40










                    • $begingroup$
                      Thank you for your well written answere! I will learn alot from it.
                      $endgroup$
                      – K-Doe
                      Feb 13 at 6:32















                    5












                    $begingroup$

                    Mathias’ answer is the correct way of solving your problem but since this is a code review site, let’s look at the rest of your code.



                    First off, your code style is fundamentally tidy and clear, so that’s very good. Now for some detailed feedback:




                     # Open Database



                    Comments such as this one are at best unnecessary and at worst harmful: the comment provides no additional information compared to the code, and at worst they are misleading. Case in point, what happens if the file SQLite_Test.db doesn’t exist yet? Your comment gives no indication — it implies that the file must exist. But SQLite will actually happily create it for you if it doesn’t exist yet.



                    Jessica Baker has written a very good article explaining how to write comments well. I strongly urge anyone to read it in its entirety.




                     c = conn.cursor()



                    Your code doesn’t need database cursors: Although the subsequent code uses c, it could use conn instead.




                     i = 0



                    This initialisation is redundant: range(…) does it for you.




                     for i in range(200):



                    Why 200? Avoid hard-coding “magic” numbers. If this is the size of your database table, don’t put the number in code, compute it from the database. If the number is arbitrary (for testing, say), assign it to a variable stating thus.




                     DB_Values = 
                    DB_Values = c.fetchone()



                    The first line here sets DB_Values to an empty list; the second line immediately overrides that value. The first lines is therefore unnecessary.



                    Apart from this, you should respect PEP 8 naming conventions. Lastly, db_values isn’t a very clear name: db_row might be more expressive.




                     Value1 = DB_Values[0]+200
                    Value2 = DB_Values[1]+400



                    Same comment regarding names, and same comment regarding magic numbers.




                     c.execute('''UPDATE Test SET Test2 = ?, Test3 = ? WHERE Test1= ?''', (Value1, Value2, i))
                    i += 1



                    You might think that it’s necessary to increment i to progress the loop but this isn’t the case. The increment of i fulfils no purpose, since it will be overridden by the for loop anyway.




                     # Save (commit) the changes



                    Same as above: this comment isn’t helpful, since it just repeats what the code says.




                     conn.commit()



                    If there was an exception in the preceding code, this commit will never be executed. You may have intended this; but chances are, you didn’t. It’s therefore best practice to avoid implicit resource cleanup (including database commits), and to use Python’s with statement instead.




                    Taking this in account, here’s a rewritten version of your code:



                    import sqlite3
                    import time

                    TEST_NUM_ROWS = 200
                    TEST2_INC = 200
                    TEST3_INC = 400

                    def database_test():
                    with sqlite3.connect('SQLite_Test.db') as conn:
                    for i in range(TEST_NUM_ROWS):
                    row = conn.execute(
                    'SELECT Test2, Test3 FROM Test WHERE Test1 = ?',
                    (i,)
                    ).fetchone()
                    new_row = (row[0] + TEST2_INC, row[1] + TEST3_INC)
                    conn.execute(
                    'UPDATE Test SET Test2 = ?, Test3 = ? WHERE Test1 = ?',
                    new_row + (i,)
                    )


                    start_time = time.time()
                    database_test()
                    print("--- %s seconds ---" % round((time.time() - start_time), 4))





                    share|improve this answer











                    $endgroup$








                    • 1




                      $begingroup$
                      I like this answer except how you use the context manager. There always seem to be some level of confusion about what the context manager on a DB connection is doing. To avoid people thinking that the connection will be closed after executing the with block, I usually separate the connect call from the with statement as shown in the docs and wrap the connect call in a with contextlib.closing(..) as conn if need be.
                      $endgroup$
                      – Mathias Ettinger
                      Feb 12 at 16:30











                    • $begingroup$
                      @MathiasEttinger I understand your concern but separating the initialisation from the context manager doesn’t fix the issue: every competent Python programmer knows that the resulting code is in fact equivalent. Rather, this needs to be fixed at the API level in sqlite3; e.g. as with conn.transaction() as t: ….
                      $endgroup$
                      – Konrad Rudolph
                      Feb 12 at 17:22











                    • $begingroup$
                      Well, that is an other argument to separate the with conn: call from the connect one (as it would be nearly useless to do with sqlite3.connect(...).transaction()) ;)
                      $endgroup$
                      – Mathias Ettinger
                      Feb 12 at 18:24










                    • $begingroup$
                      @MathiasEttinger If that API existed I’d agree, yes.
                      $endgroup$
                      – Konrad Rudolph
                      Feb 12 at 18:40










                    • $begingroup$
                      Thank you for your well written answere! I will learn alot from it.
                      $endgroup$
                      – K-Doe
                      Feb 13 at 6:32













                    5












                    5








                    5





                    $begingroup$

                    Mathias’ answer is the correct way of solving your problem but since this is a code review site, let’s look at the rest of your code.



                    First off, your code style is fundamentally tidy and clear, so that’s very good. Now for some detailed feedback:




                     # Open Database



                    Comments such as this one are at best unnecessary and at worst harmful: the comment provides no additional information compared to the code, and at worst they are misleading. Case in point, what happens if the file SQLite_Test.db doesn’t exist yet? Your comment gives no indication — it implies that the file must exist. But SQLite will actually happily create it for you if it doesn’t exist yet.



                    Jessica Baker has written a very good article explaining how to write comments well. I strongly urge anyone to read it in its entirety.




                     c = conn.cursor()



                    Your code doesn’t need database cursors: Although the subsequent code uses c, it could use conn instead.




                     i = 0



                    This initialisation is redundant: range(…) does it for you.




                     for i in range(200):



                    Why 200? Avoid hard-coding “magic” numbers. If this is the size of your database table, don’t put the number in code, compute it from the database. If the number is arbitrary (for testing, say), assign it to a variable stating thus.




                     DB_Values = 
                    DB_Values = c.fetchone()



                    The first line here sets DB_Values to an empty list; the second line immediately overrides that value. The first lines is therefore unnecessary.



                    Apart from this, you should respect PEP 8 naming conventions. Lastly, db_values isn’t a very clear name: db_row might be more expressive.




                     Value1 = DB_Values[0]+200
                    Value2 = DB_Values[1]+400



                    Same comment regarding names, and same comment regarding magic numbers.




                     c.execute('''UPDATE Test SET Test2 = ?, Test3 = ? WHERE Test1= ?''', (Value1, Value2, i))
                    i += 1



                    You might think that it’s necessary to increment i to progress the loop but this isn’t the case. The increment of i fulfils no purpose, since it will be overridden by the for loop anyway.




                     # Save (commit) the changes



                    Same as above: this comment isn’t helpful, since it just repeats what the code says.




                     conn.commit()



                    If there was an exception in the preceding code, this commit will never be executed. You may have intended this; but chances are, you didn’t. It’s therefore best practice to avoid implicit resource cleanup (including database commits), and to use Python’s with statement instead.




                    Taking this in account, here’s a rewritten version of your code:



                    import sqlite3
                    import time

                    TEST_NUM_ROWS = 200
                    TEST2_INC = 200
                    TEST3_INC = 400

                    def database_test():
                    with sqlite3.connect('SQLite_Test.db') as conn:
                    for i in range(TEST_NUM_ROWS):
                    row = conn.execute(
                    'SELECT Test2, Test3 FROM Test WHERE Test1 = ?',
                    (i,)
                    ).fetchone()
                    new_row = (row[0] + TEST2_INC, row[1] + TEST3_INC)
                    conn.execute(
                    'UPDATE Test SET Test2 = ?, Test3 = ? WHERE Test1 = ?',
                    new_row + (i,)
                    )


                    start_time = time.time()
                    database_test()
                    print("--- %s seconds ---" % round((time.time() - start_time), 4))





                    share|improve this answer











                    $endgroup$



                    Mathias’ answer is the correct way of solving your problem but since this is a code review site, let’s look at the rest of your code.



                    First off, your code style is fundamentally tidy and clear, so that’s very good. Now for some detailed feedback:




                     # Open Database



                    Comments such as this one are at best unnecessary and at worst harmful: the comment provides no additional information compared to the code, and at worst they are misleading. Case in point, what happens if the file SQLite_Test.db doesn’t exist yet? Your comment gives no indication — it implies that the file must exist. But SQLite will actually happily create it for you if it doesn’t exist yet.



                    Jessica Baker has written a very good article explaining how to write comments well. I strongly urge anyone to read it in its entirety.




                     c = conn.cursor()



                    Your code doesn’t need database cursors: Although the subsequent code uses c, it could use conn instead.




                     i = 0



                    This initialisation is redundant: range(…) does it for you.




                     for i in range(200):



                    Why 200? Avoid hard-coding “magic” numbers. If this is the size of your database table, don’t put the number in code, compute it from the database. If the number is arbitrary (for testing, say), assign it to a variable stating thus.




                     DB_Values = 
                    DB_Values = c.fetchone()



                    The first line here sets DB_Values to an empty list; the second line immediately overrides that value. The first lines is therefore unnecessary.



                    Apart from this, you should respect PEP 8 naming conventions. Lastly, db_values isn’t a very clear name: db_row might be more expressive.




                     Value1 = DB_Values[0]+200
                    Value2 = DB_Values[1]+400



                    Same comment regarding names, and same comment regarding magic numbers.




                     c.execute('''UPDATE Test SET Test2 = ?, Test3 = ? WHERE Test1= ?''', (Value1, Value2, i))
                    i += 1



                    You might think that it’s necessary to increment i to progress the loop but this isn’t the case. The increment of i fulfils no purpose, since it will be overridden by the for loop anyway.




                     # Save (commit) the changes



                    Same as above: this comment isn’t helpful, since it just repeats what the code says.




                     conn.commit()



                    If there was an exception in the preceding code, this commit will never be executed. You may have intended this; but chances are, you didn’t. It’s therefore best practice to avoid implicit resource cleanup (including database commits), and to use Python’s with statement instead.




                    Taking this in account, here’s a rewritten version of your code:



                    import sqlite3
                    import time

                    TEST_NUM_ROWS = 200
                    TEST2_INC = 200
                    TEST3_INC = 400

                    def database_test():
                    with sqlite3.connect('SQLite_Test.db') as conn:
                    for i in range(TEST_NUM_ROWS):
                    row = conn.execute(
                    'SELECT Test2, Test3 FROM Test WHERE Test1 = ?',
                    (i,)
                    ).fetchone()
                    new_row = (row[0] + TEST2_INC, row[1] + TEST3_INC)
                    conn.execute(
                    'UPDATE Test SET Test2 = ?, Test3 = ? WHERE Test1 = ?',
                    new_row + (i,)
                    )


                    start_time = time.time()
                    database_test()
                    print("--- %s seconds ---" % round((time.time() - start_time), 4))






                    share|improve this answer














                    share|improve this answer



                    share|improve this answer








                    edited Feb 12 at 14:13

























                    answered Feb 12 at 14:08









                    Konrad RudolphKonrad Rudolph

                    4,9811631




                    4,9811631







                    • 1




                      $begingroup$
                      I like this answer except how you use the context manager. There always seem to be some level of confusion about what the context manager on a DB connection is doing. To avoid people thinking that the connection will be closed after executing the with block, I usually separate the connect call from the with statement as shown in the docs and wrap the connect call in a with contextlib.closing(..) as conn if need be.
                      $endgroup$
                      – Mathias Ettinger
                      Feb 12 at 16:30











                    • $begingroup$
                      @MathiasEttinger I understand your concern but separating the initialisation from the context manager doesn’t fix the issue: every competent Python programmer knows that the resulting code is in fact equivalent. Rather, this needs to be fixed at the API level in sqlite3; e.g. as with conn.transaction() as t: ….
                      $endgroup$
                      – Konrad Rudolph
                      Feb 12 at 17:22











                    • $begingroup$
                      Well, that is an other argument to separate the with conn: call from the connect one (as it would be nearly useless to do with sqlite3.connect(...).transaction()) ;)
                      $endgroup$
                      – Mathias Ettinger
                      Feb 12 at 18:24










                    • $begingroup$
                      @MathiasEttinger If that API existed I’d agree, yes.
                      $endgroup$
                      – Konrad Rudolph
                      Feb 12 at 18:40










                    • $begingroup$
                      Thank you for your well written answere! I will learn alot from it.
                      $endgroup$
                      – K-Doe
                      Feb 13 at 6:32












                    • 1




                      $begingroup$
                      I like this answer except how you use the context manager. There always seem to be some level of confusion about what the context manager on a DB connection is doing. To avoid people thinking that the connection will be closed after executing the with block, I usually separate the connect call from the with statement as shown in the docs and wrap the connect call in a with contextlib.closing(..) as conn if need be.
                      $endgroup$
                      – Mathias Ettinger
                      Feb 12 at 16:30











                    • $begingroup$
                      @MathiasEttinger I understand your concern but separating the initialisation from the context manager doesn’t fix the issue: every competent Python programmer knows that the resulting code is in fact equivalent. Rather, this needs to be fixed at the API level in sqlite3; e.g. as with conn.transaction() as t: ….
                      $endgroup$
                      – Konrad Rudolph
                      Feb 12 at 17:22











                    • $begingroup$
                      Well, that is an other argument to separate the with conn: call from the connect one (as it would be nearly useless to do with sqlite3.connect(...).transaction()) ;)
                      $endgroup$
                      – Mathias Ettinger
                      Feb 12 at 18:24










                    • $begingroup$
                      @MathiasEttinger If that API existed I’d agree, yes.
                      $endgroup$
                      – Konrad Rudolph
                      Feb 12 at 18:40










                    • $begingroup$
                      Thank you for your well written answere! I will learn alot from it.
                      $endgroup$
                      – K-Doe
                      Feb 13 at 6:32







                    1




                    1




                    $begingroup$
                    I like this answer except how you use the context manager. There always seem to be some level of confusion about what the context manager on a DB connection is doing. To avoid people thinking that the connection will be closed after executing the with block, I usually separate the connect call from the with statement as shown in the docs and wrap the connect call in a with contextlib.closing(..) as conn if need be.
                    $endgroup$
                    – Mathias Ettinger
                    Feb 12 at 16:30





                    $begingroup$
                    I like this answer except how you use the context manager. There always seem to be some level of confusion about what the context manager on a DB connection is doing. To avoid people thinking that the connection will be closed after executing the with block, I usually separate the connect call from the with statement as shown in the docs and wrap the connect call in a with contextlib.closing(..) as conn if need be.
                    $endgroup$
                    – Mathias Ettinger
                    Feb 12 at 16:30













                    $begingroup$
                    @MathiasEttinger I understand your concern but separating the initialisation from the context manager doesn’t fix the issue: every competent Python programmer knows that the resulting code is in fact equivalent. Rather, this needs to be fixed at the API level in sqlite3; e.g. as with conn.transaction() as t: ….
                    $endgroup$
                    – Konrad Rudolph
                    Feb 12 at 17:22





                    $begingroup$
                    @MathiasEttinger I understand your concern but separating the initialisation from the context manager doesn’t fix the issue: every competent Python programmer knows that the resulting code is in fact equivalent. Rather, this needs to be fixed at the API level in sqlite3; e.g. as with conn.transaction() as t: ….
                    $endgroup$
                    – Konrad Rudolph
                    Feb 12 at 17:22













                    $begingroup$
                    Well, that is an other argument to separate the with conn: call from the connect one (as it would be nearly useless to do with sqlite3.connect(...).transaction()) ;)
                    $endgroup$
                    – Mathias Ettinger
                    Feb 12 at 18:24




                    $begingroup$
                    Well, that is an other argument to separate the with conn: call from the connect one (as it would be nearly useless to do with sqlite3.connect(...).transaction()) ;)
                    $endgroup$
                    – Mathias Ettinger
                    Feb 12 at 18:24












                    $begingroup$
                    @MathiasEttinger If that API existed I’d agree, yes.
                    $endgroup$
                    – Konrad Rudolph
                    Feb 12 at 18:40




                    $begingroup$
                    @MathiasEttinger If that API existed I’d agree, yes.
                    $endgroup$
                    – Konrad Rudolph
                    Feb 12 at 18:40












                    $begingroup$
                    Thank you for your well written answere! I will learn alot from it.
                    $endgroup$
                    – K-Doe
                    Feb 13 at 6:32




                    $begingroup$
                    Thank you for your well written answere! I will learn alot from it.
                    $endgroup$
                    – K-Doe
                    Feb 13 at 6:32

















                    draft saved

                    draft discarded
















































                    Thanks for contributing an answer to Code Review Stack Exchange!


                    • Please be sure to answer the question. Provide details and share your research!

                    But avoid


                    • Asking for help, clarification, or responding to other answers.

                    • Making statements based on opinion; back them up with references or personal experience.

                    Use MathJax to format equations. MathJax reference.


                    To learn more, see our tips on writing great answers.




                    draft saved


                    draft discarded














                    StackExchange.ready(
                    function ()
                    StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f213288%2fpython-update-sqlite-db-values%23new-answer', 'question_page');

                    );

                    Post as a guest















                    Required, but never shown





















































                    Required, but never shown














                    Required, but never shown












                    Required, but never shown







                    Required, but never shown

































                    Required, but never shown














                    Required, but never shown












                    Required, but never shown







                    Required, but never shown






                    Popular posts from this blog

                    Peggy Mitchell

                    Palaiologos

                    The Forum (Inglewood, California)