HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Adding a list of brokers to a local SQLite database

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
localsqliteaddingbrokersdatabaselist

Problem

I have this piece of Java (Android) code that adds a list of brokers to a local SQLite database as one single SQL instruction.

public void Add(List brokers)
{
    if(brokers == null || brokers.size() == 0)
      return;

    String sql = "INSERT INTO " + TABLE_NAME + " SELECT " + brokers.get(0).getId() + " AS '" + COLUMN_BROKERID + "', "+ brokers.get(0).getOfficeId() + " AS '" + COLUMN_OFFICEID +  "', '"+ brokers.get(0).getName() + "' AS '" + COLUMN_NAME +  "', "+ brokers.get(0).getSuccessRate() + " AS '" + COLUMN_SUCCESSRATE +  "'";

    for(int i=1; i<brokers.size(); i++)
    {
        sql = sql + " UNION SELECT " + brokers.get(i).getId() + ", " + brokers.get(i).getOfficeId() + ", '" + brokers.get(i).getName() + "', " + brokers.get(i).getSuccessRate();
    }

    databaseManager.ExecuteNonQuery(sql);
 }


But what slows this down a lot is the change in the string 'sql'. The last line, which is a call to ExecuteNonQuery(), takes a millisecond, but the above takes a lot. How can I speed this up?

databaseManager is an interface that is implemented by a class of SQLiteOpenHelper and simply executes an SQL statement (but don't take this into account; it doesn't take any time as much as the rest of the code).

Solution

A classicel one. String concatenation with the + operator is very inefficient, because it creates a new copy of the string every time.

See for example item 51 in Effective Java from Joshua Bloch or here: http://www.javapractices.com/topic/TopicAction.do?Id=4

Try to change it to StringBuilder:

public void Add(List brokers)
{
    if(brokers == null || brokers.size() == 0)
      return;

    StringBuilder query = new StringBuilder(brokers.size() * 50); // size is a guess, could be furhter investigated
    query.append("INSERT INTO ").append(TABLE_NAME).append(" SELECT ").append(brokers.get(0).getId()).append(" AS '").append(COLUMN_BROKERID).append("', ").append(brokers.get(0).getOfficeId()).append(" AS '").append(COLUMN_OFFICEID).append("', '").append(brokers.get(0).getName()).append("' AS '").append(COLUMN_NAME).append("', ").append(brokers.get(0).getSuccessRate()).append(" AS '").append(COLUMN_SUCCESSRATE).append("'"); // it is not that important to change this line. this will probably be done by the java compiler anyway

    for(int i=1; i<brokers.size(); i++)
        query.append(" UNION SELECT ").append(brokers.get(i).getId()).append(", ").append(brokers.get(i).getOfficeId()).append(", '").append(brokers.get(i).getName()).append("', ").append(brokers.get(i).getSuccessRate());

    databaseManager.ExecuteNonQuery(query.toString(););
 }

Code Snippets

public void Add(List<Broker> brokers)
{
    if(brokers == null || brokers.size() == 0)
      return;

    StringBuilder query = new StringBuilder(brokers.size() * 50); // size is a guess, could be furhter investigated
    query.append("INSERT INTO ").append(TABLE_NAME).append(" SELECT ").append(brokers.get(0).getId()).append(" AS '").append(COLUMN_BROKERID).append("', ").append(brokers.get(0).getOfficeId()).append(" AS '").append(COLUMN_OFFICEID).append("', '").append(brokers.get(0).getName()).append("' AS '").append(COLUMN_NAME).append("', ").append(brokers.get(0).getSuccessRate()).append(" AS '").append(COLUMN_SUCCESSRATE).append("'"); // it is not that important to change this line. this will probably be done by the java compiler anyway

    for(int i=1; i<brokers.size(); i++)
        query.append(" UNION SELECT ").append(brokers.get(i).getId()).append(", ").append(brokers.get(i).getOfficeId()).append(", '").append(brokers.get(i).getName()).append("', ").append(brokers.get(i).getSuccessRate());

    databaseManager.ExecuteNonQuery(query.toString(););
 }

Context

StackExchange Code Review Q#24980, answer score: 4

Revisions (0)

No revisions yet.