3
\$\begingroup\$

I am a newbie in back-end developing. I know that the biggest part of back-end development is interaction with database.

I established a connection and the code is 100% working, but I am wondering if it is well optimized. There is one function where I make double insertion into the database but before I commit this insertion I make two requests to the database to determine whether I should insert or not.

from flask import Flask, jsonify, request, url_for
import MySQLdb

def connection():
    conn = MySQLdb.connect(host='somehost',
                           user='root',
                           passwd='some password',
                           db='users_addresses',
                           port=3306
                           )
    c = conn.cursor()
    return c, conn

def processing():
    # some actions
    try:
        c, conn = connection()
        c = conn.cursor()
        mc_address_existence, id = mc_address_exist(c, conn, mc_address=box_id)
        if mc_address_existence and not login_exist(c, conn, login):

            c.execute("INSERT INTO accounts_info (uid, first_name, second_name, email, phone_number) "
                      "VALUES ({},"
                            " '{}',"
                            " '{}', "
                            "'{}',"
                            " {} ) ".format(id, first_name, second_name, email, phone_number)
                      )

            c.execute("INSERT INTO logins_passwords (uid, login, password) "
                      "VALUES ({},"
                      "'{}',"
                      " '{}' ) ".format(id, login, password)
                      )
            conn.commit()
            c.close()
            conn.close()
            return jsonify({'result': True})
        return jsonify({'result': False})
    except Exception as e:
        return str(e)

And here are two functions for determining whether to make the insertion or not:

def mc_address_exist(c, conn, mc_address):
    # Should I wrap it for try/except?
    # c = conn.cursor()
    c.execute("SELECT uid, mc_address "
              "FROM boxes_id "
              "WHERE mc_address='{}' ".format(mc_address))
    res = c.fetchall()
    # c.close() Should I close?
    # conn.close()
    if res:
        return True, res[0][0]
    return False, 0


def login_exist(c, conn, login):
    # should I wrap it for try/except?
    c.execute("SELECT * "
              "FROM logins_passwords "
              "WHERE login='{}' ".format(login))
    res = c.fetchall()
    # c.close() should I close it or not?
    # conn.close()
    if res:
        return True
    return False

It is working code, but maybe not optimized. Here are three questions:

  • Should I close the cursor (conn, c) in the checking functions?
  • Should I wrap the database's selection checking functions into try/except?
  • Maybe I should use with as instead?
\$\endgroup\$
1
\$\begingroup\$
def connection():

This is misnamed. Please call it cursor_connection(), as that is the tuple it returns. Some folks would prefer the identifier curs over the rather ambiguous c, as "cursor" and "connection" both start with that letter.

    c, conn = connection()
    c = conn.cursor()

Ok, that 2nd line is just bizarre. Please delete it, as the function already accomplished that task.

Nice little INSERT you have there. Be a shame if some punctuation crept into it: https://www.xkcd.com/327/

except Exception as e:

What is your concern, here? Might be better to elide the try / catch altogether, since there isn't really any "error recovery code" here.

Rather than having mc_address_exist() return a tuple, it would be more natural to return a (potentially empty) list, or return a scalar that potentially is None (assuming the uid column is declared NOT NULL). It's not clear why you asked the DB to retrieve mc_address, since you just discard it.

In login_exist(), rather than asking for *, it would be a cheaper query to just select count(*).

You're not leaking anything with that cursor, so don't worry about closing it. You are correctly closing the underlying TCP connection. Your try/catch handling appears to be adequate for your needs at present, so no change needed.

\$\endgroup\$
1
\$\begingroup\$

J_H provides some good feedback. In addition, I would suggest that you combine your SQL statements into a single statement, to reduce traffic back-and-forth to the database.

Your code is really asking "do these entries exist in the tables?" So ask that! Use a nested select to ask your two different questions, and just do a COUNT() on them. If they're 1 and 1-or-more, you're good to proceed:

SELECT COUNT(logins) == 1 AND COUNT(addresses) >= 1
  FROM 
    (SELECT uid FROM boxes_id WHERE mc_address='{}') AS addresses,
    (SELECT login FROM logins_passwords WHERE login='{}') AS logins
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.