0
\$\begingroup\$

I have this piece of code in Python 2.7: As you can see is very hard to read the SQL query, what is the best way to re-write it?

    try:
        if title and url:
            sql_query = "INSERT INTO news (title, author, description, " \
                        "content, url, url_to_image, source_id, source, " \
                        "campaign, published_at, score, magnitude, sentiment, " \
                        "inserted_at ) VALUES ('%s','%s','%s','%s'," \
                        "'%s','%s','%s','%s','%s','%s', %s, %s, '%s', '%s')" \
                        % ( title.replace("'", "''"),
                            author.replace("'", "''"),
                            description.replace("'", "''"),
                            content[:settings.content_size].replace("'", "''"),
                            url,
                            url_to_image,
                            source_id,
                            source,
                            campaign,
                            published_at,
                            score,
                            magnitude,
                            sentiment,
                            DB_NOW)
            db = Db.Db()
            db.initialize(dsn=settings.SQLALCHEMY_DSN)
            return db.insert_content(sql_query, 'news_id')
    except psycopg2.ProgrammingError as exception:
        log.exception(exception)`enter code here`
\$\endgroup\$
6
  • \$\begingroup\$ which python version? \$\endgroup\$ Commented Oct 9, 2018 at 7:44
  • 1
    \$\begingroup\$ Python version 2.7 \$\endgroup\$
    – gogasca
    Commented Oct 9, 2018 at 7:47
  • 3
    \$\begingroup\$ Can you show your imports so we know what library you are using? \$\endgroup\$ Commented Oct 9, 2018 at 8:29
  • \$\begingroup\$ This question is incomplete. To help reviewers give you better answers, please edit to add sufficient context to your question. The more you tell us about what your code does and what the purpose of doing that is, the easier it will be for reviewers to help you. Questions should include a description of what the code does. "SQL query" describes the mechanism you're using, but not the motive - why do you need this query, and what are the data being queried? Also, your code seems to be lacking some essential definitions or imports. \$\endgroup\$ Commented Oct 9, 2018 at 12:22
  • \$\begingroup\$ The current question title, which states your concerns about the code, is too general to be useful here. Please edit to the site standard, which is for the title to simply state the task accomplished by the code. Please see How to get the best value out of Code Review: Asking Questions for guidance on writing good question titles. \$\endgroup\$ Commented Oct 9, 2018 at 12:22

1 Answer 1

2
\$\begingroup\$

You can use multiline sting """ and string.format

try:
    if title and url:
        sql_query = """
        INSERT INTO news (
        title, author, description, content, url, url_to_image, source_id,
        source, campaign, published_at, score, magnitude, sentiment, inserted_at
         ) VALUES (
         '{title}','{author}','{description}','{content}', '{url}',
         '{url_to_image}','{source_id}','{source}', '{campaign}',
         '{published_at}', {score}, {magnitude}, '{sentiment}', '{DB_NOW}'
         )
        """.format(
            title=title.replace("'", "''"),
            author=author.replace("'", "''"),
            description=description.replace("'", "''"),
            content=content[: settings.content_size].replace("'", "''"),
            url=url,
            url_to_image=url_to_image,
            source_id=source_id,
            source=source,
            campaign=campaign,
            published_at=published_at,
            score=score,
            magnitude=magnitude,
            sentiment=sentiment,
            DB_NOW=DB_NOW,
        )

        db = Db.Db()
        db.initialize(dsn=settings.SQLALCHEMY_DSN)
        return db.insert_content(sql_query, "news_id")
except psycopg2.ProgrammingError as exception:
    log.exception(exception)`enter code here`

If you'd use python 3.6+, you could even simpler use f-strings

string interpolation

As noted by @Ludisposed, string interpolation is not the way to go link Since it is unclear what the Db.Db is, I assume it is something you wrote yourself. You will have to adapt the insert_content code to accept the arguments, and then do something like this:

try:
    if title and url:
        sql_query = """
        INSERT INTO news (
        title, author, description, content, url, url_to_image, source_id,
        source, campaign, published_at, score, magnitude, sentiment, inserted_at
         ) VALUES (
         %(title)s,%(author)s,%(description)s,%(content)s, %(url)s,
         %(url_to_image)s,%(source_id)s,%(source)s, %(campaign)s,
         %(published_at)s, %(score)s, %(magnitude)s, %(sentiment)s, %(DB_NOW)s
         )
        """
        arguments = dict(
            title=title,
            author=author,
            description=description,
            content=content[: settings.content_size],
            url=url,
            url_to_image=url_to_image,
            source_id=source_id,
            source=source,
            campaign=campaign,
            published_at=published_at,
            score=score,
            magnitude=magnitude,
            sentiment=sentiment,
            DB_NOW=DB_NOW,
        )

        db = Db.Db()
        db.initialize(dsn=settings.SQLALCHEMY_DSN)
        return db.insert_content(sql_query, arguments, "news_id")
except psycopg2.ProgrammingError as exception:
    log.exception(exception)
\$\endgroup\$
4
  • 2
    \$\begingroup\$ Don't use string interpolation with SQL queries, this is an SQLinjection waiting to happen. Let the SQL driver handle that with magic %s as the OP does \$\endgroup\$
    – Ludisposed
    Commented Oct 9, 2018 at 9:12
  • \$\begingroup\$ At this moment, the OP uses string interpolation before passing it to the db river. I admit I don't know much about db code, I just suggested a different method to do the string interpolation. \$\endgroup\$ Commented Oct 9, 2018 at 9:50
  • \$\begingroup\$ One more comment, I think %(title)s should be %(str)s just the type instead of the name \$\endgroup\$
    – Ludisposed
    Commented Oct 9, 2018 at 10:56
  • \$\begingroup\$ I think you can use named arguments. The example just uses those types. Named arguments are supported too using %(name)s placeholders in the query and specifying the values into a mapping. Using named arguments allows to specify the values in any order and to repeat the same value in several places in the query: \$\endgroup\$ Commented Oct 9, 2018 at 12:41

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