Grouping of related variables and operations?

Question

In Code Complete, chapter 10, it is advised to group related statements, and the following example is given:

void SummarizeData(...) {
    ...
    GetOldData( oldData, &numOldData );
    GetNewData( newData, &numNewData );
    totalOldData = Sum( oldData, numOldData );
    totalNewData = Sum( newData, numNewData );
    PrintOldDataSummary( oldData, totalOldData, numOldData );
    PrintNewDataSummary( newData, totalNewData, numNewData );
    SaveOldDataSummary( totalOldData, numOldData );
    SaveNewDataSummary( totalNewData, numNewData );
    ...
}

It is stated that such grouping and concurrent processing is bad design, and instead gives something more separated:

void SummarizeData(...) {
    GetOldData( oldData, &numOldData );
    totalOldData = Sum( oldData, numOldData );
    PrintOldDataSummary( oldData, totalOldData, numOldData );
    SaveOldDataSummary( totalOldData, numOldData );
    ...
    GetNewData( newData, &numNewData );
    totalNewData = Sum( newData, numNewData );
    PrintNewDataSummary( newData, totalNewData, numNewData );
    SaveNewDataSummary( totalNewData, numNewData );
    ...
}

I do agree that the second approach is easier to read and to understand, and offers cleaner-looking code, at least from my own perspective. So, my question is, are there any disadvantages with the second approach? For example, one possible issue that I could think of is with temporary connections to databases and such:

void SummarizeData(...) {
    ...
    externalDataStore.open();
    externalDataStore.save(oldData, numOldData);
    externalDataStore.save(newData, numNewData);
    externalDataStore.close();
    ...
}

This first approach would complete both save operations in one open/close cycle. However, with the second approach...

void SummarizeData(...) {
    ...
    externalDataStore.open();
    externalDataStore.save(oldData, numOldData);
    externalDataStore.close();
    ...
    externalDataStore.open();
    externalDataStore.save(newData, numNewData);
    externalDataStore.close();
    ...
}

You have to open and close the connection for each operation. This seems wasteful, but I have no idea how it affects performance in practice.

Sorry for the unnecessarily long question...

score 1 · Accepted Answer · answered Aug 26 '11 at 15:05

I haven't gotten to Chapter 10 in Code Complete just yet (a few more evenings ought to do it!) but I think the main point here is to group your lines of code in a logical and easily readable way, without affecting program functionality. In other words, clean it up and rearrange it as much as possible, but stop as soon as it starts to actually affect behaviour.

In your example, we should keep in mind that "Premature optimization is the root of all evil," but I think we can still safely assume that you shouldn't be closing the connection if you're about to open it again right away again, since those two actions literally cancel each other out. As a general rule, any connections should be opened only right before the first time you need them, and closed immediately after the last time they're used, for simplicity's sake.

Thanks for the pointers. Code Complete has a lot of information to think about, but, as with anything, you still can't take it for face-value. — voithos, Sep 02 '11 at 18:10

score 0 · Answer 2 · answered Sep 02 '11 at 18:08

I was bored, so I tried doing a proof-of-concept speed test in Python using Sqlite (which I realize is not the best way to do it).

First, the base test of 50,000 iterations, opening and closing the connection after every iteration.

#!/usr/bin/env python

import sqlite3

class Creature(object):
    legs = 0

    eyes = 'monocular'

    kind = ''

conn = sqlite3.connect(':memory:')

c = conn.cursor()
c.execute('''create table testtable
        (date text, legs text, eyes text, kind text)''')
conn.commit()
c.close()

for i in range(50000):
    c = conn.cursor()

    creature1 = Creature()
    creature1.legs = 5
    creature1.eyes = 'monocular'
    creature1.kind = 'mungy'
    c.execute('insert into testtable values (?,?,?,?)', ('today', str(creature1.legs), creature1.eyes, creature1.kind))

    creature2 = Creature()
    creature2.legs = 3
    creature2.eyes = 'binocular'
    creature2.kind = 'thingy'
    c.execute('insert into testtable values (?,?,?,?)', ('today', str(creature2.legs), creature2.eyes, creature2.kind))

    creature3 = Creature()
    creature3.legs = 3
    creature3.eyes = 'monocular'
    creature3.kind = 'pungy'    
    c.execute('insert into testtable values (?,?,?,?)', ('today', str(creature3.legs), creature3.eyes, creature3.kind))

    conn.commit()
    c.close()

And now, 50,000 iterations, but without closing the connection.

#!/usr/bin/env python

import sqlite3

class Creature(object):
    legs = 0

    eyes = 'monocular'

    kind = ''

conn = sqlite3.connect(':memory:')

c = conn.cursor()
c.execute('''create table testtable
        (date text, legs text, eyes text, kind text)''')
conn.commit()
c.close()

c = conn.cursor()
for i in range(50000):

    creature1 = Creature()
    creature1.legs = 5
    creature1.eyes = 'monocular'
    creature1.kind = 'mungy'

    creature2 = Creature()
    creature2.legs = 3
    creature2.eyes = 'binocular'
    creature2.kind = 'thingy'

    creature3 = Creature()
    creature3.legs = 3
    creature3.eyes = 'monocular'
    creature3.kind = 'pungy'

    c.execute('insert into testtable values (?,?,?,?)', ('today', str(creature1.legs), creature1.eyes, creature1.kind))
    c.execute('insert into testtable values (?,?,?,?)', ('today', str(creature2.legs), creature2.eyes, creature2.kind))
    c.execute('insert into testtable values (?,?,?,?)', ('today', str(creature3.legs), creature3.eyes, creature3.kind))

    conn.commit()
c.close()

The results?

First method: Average 2.264s
Second method: Average 2.157s

So, it makes a difference, albeit a negligible one.

And there you have it.

Definitely agree with what Stephane said, though.

Grouping of related variables and operations?

2 Answers2