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\$
  • \$\begingroup\$ which python version? \$\endgroup\$ – Maarten Fabré Oct 9 '18 at 7:44
  • 1
    \$\begingroup\$ Python version 2.7 \$\endgroup\$ – gogasca Oct 9 '18 at 7:47
  • 3
    \$\begingroup\$ Can you show your imports so we know what library you are using? \$\endgroup\$ – 301_Moved_Permanently Oct 9 '18 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\$ – Toby Speight Oct 9 '18 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\$ – Toby Speight Oct 9 '18 at 12:22
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\$
  • 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 Oct 9 '18 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\$ – Maarten Fabré Oct 9 '18 at 9:50
  • \$\begingroup\$ One more comment, I think %(title)s should be %(str)s just the type instead of the name \$\endgroup\$ – Ludisposed Oct 9 '18 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\$ – Maarten Fabré Oct 9 '18 at 12:41

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