HOME BLOG ARCHIVE TAGS

Some Notes for Beginners

April 28, 2015

Nothing is worst for morale than having to change messy code bases. And although software engineering is a well established discipline, some developers disrespect simple and effective programming principles.

From time to time, it’s good to have related notes around (for revision/reference). So here we go. Boredom disclaimer: they’re mostly directed towards beginners.

SHORTCUTS + CODE SMELLS

It’s pretty common to see good coding practices violations when refactoring poorly structured projects. Specially when shortcuts are taken. They seem to buy us some time in the short term, but really produce inferior work on the long run.

DRY: THE MOST TALKED PRINCIPLE (MAYBE THE LEAST PRACTICED)

  • Repetitions are bad, right?

Everybody knowns that. It’s common sense. Even kids understand the implications of doing stuff more than one time. So why duplicated code is widely tolerated?

For those (rare) programmers that don’t identify this core “DRY” principle, it means don’t repeat yourself.

Applying it systematically is much harder than theory suggests. The key concept is systematic. E.g., the copy and paste programming below is obvious (adapted from real codebase):

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
    short b_len = 0;
    short h_len = 0;

    if ( in[i] > K1 ) {
        if ( (in[i++] & K2) == 1 ) {
            h_len = x;
            b_len = in[i++];
        } else {
            h_len  = y;
            b_len  = in[i++] << 8;
            b_len += in[i++];
        }
    } else {
        h_len = z;
        b_len = in[i++]; 
    }

    if ( in_len != (h_len + b_len) ) {
        // err
    }

    // (...)

    short e_len = 0;

    if ( in[i] > K1 ) {
        if ( (in[i++] & K2) == 1 ) {
            e_len = in[i++];
        } else {
            e_len  = in[i++] << 8;
            e_len += in[i++];
        }
    } else {
        e_len = in[i++];
    }

But would you be able to spot such blocks, hidden inside tons of lines?

  • Debugging code and metadata can add complexity to the game

It’s easy to duplicate and waste resources when commenting and asserting. I.e., it’s not a good idea to do this without appropriate reasons:

1
2
3
4
    // foo existence is guaranteed to be in this range;
    if ( x < K1 || x > K2 ) {
        // err;
    }

The knowledge of foo is spread; a better approach to couple related information in one place:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
    static bool is_foo_in_range(size_t x)
    {
        return ( x >= K1 && x <= K2 );
    }

    // (...)

    if ( !is_foo_in_range(x)  ) {
        // err;
    }

For the same reason, it’s harmful to duplicate checks inside debugging code:

1
2
3
4
5
6
    assert( x >= K1 && x <= K2 );

    // (...)
    if ( x < K1 || x > K2 ) {
        // err;
    }
  • Small helper functions can increase code quality

As another good practice is to comment asserts, it’s also almost guaranteed that DRY violations will be introduced this way. One alternative for aforementioned issues:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
    static bool is_foo_in_range(size_t x)
    {
        assert( x >= K1 );
        assert( x <= K2 );
        return ( x >= K1 && x <= K2 );
    }

    assert( is_foo_in_range(x) );

    // (...)
    if ( !is_foo_in_range(x)  ) {
        // err;
    }

THERE’S A CASE FOR BEAUTY IN THE WORLD

  • Extra point in favor of DRY: repetitions and ugliness walk side by side

For one reason or another, programming is sometimes regarded as software craftsmanship. A common “friction” between C++ and callbacks may illustrate better this “artistic” point of view:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
    // foo.h
    class CFoo
    {
     public:
        static unsigned _stdcall run(void *pFoo);
        // (...)
    };

    // foo.cpp
    unsigned _stdcall CFoo::run(void *pFoo)
    {
        CFoo *pThis = (CFoo *) pFoo;

        while ( pThis->wait() ) {
            pThis->lock();
            pThis->bar();
            pThis->unlock();
            // (...)
        }

        return pThis->status();
    }

    // main.cpp
    CFoo foo;
    _beginthreadex(NULL, 0, CFoo::run, &foo, 0, &tid);
    // (...)

The repetitions seem necessary. After all, we’re working in the realm of OOP, while a low-level API expects raw callback functions. A pattern so usual, that a small change allows the removal of all ‘pThis->’ noise:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
    // foo.h
    class CFoo
    {
     private:
        unsigned run_(void);

     public:
        static unsigned _stdcall run(void *arg);
        {
            return ((CFoo *)arg)->run_();
        }
        // (...)
    };

    // foo.cpp
    unsigned CFoo::run_()
    {
        while ( wait() ) {
            lock();
            bar();
            unlock();
            // (...)
        }

        return status();
    }

    // main.cpp
    CFoo foo;
    _beginthreadex(NULL, 0, CFoo::run, &foo, 0, &tid);
    // (...)

From a legacy app:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
    if ( e_target < K0 )
        in[i++] = e_target;
    else
        if ( e_target < K1 ) {
            in[i++] = K2;
            in[i++] = e_target;
        } else {
            in[i++] = K3;
            in[i++] = (e_target >> 8);
            in[i++] = (e_target & K3);
        }

Even though indent styles are religious matter, I try to follow and recommend Linux coding style:

(…) if, however, at least one of the sub-statements in an ‘if-else’ statement requires braces, then both sub-statements should be wrapped inside curly braces (…)

It’s easy to introduce subtle bugs in such beasts. Just ask Apple (nice wrap-up here).

HACKS HAVE THEIR PLACE AND TIME

I’m all for hacks. Everybody likes creative solutions. But they have their place and time. Today’s well placed hack may become tomorrow’s (hated) kludge/nightmare.

CONCLUSIONS

Avoid repetitions. Refactor like crazy. Strive to produce beautiful code. And don’t be fooled by silly things (e.g., the rule of 3).

It may make sense to duplicate code sometimes (like exceptions that confirm the rule). Even then, it’s a huge maybe.