How can I use whitespaces to improve code readability

[abhishekm71]

Hi!

I have read code written by experienced programmers on this site and elsewhere.

What I have noticed is that sometimes it takes much more effort to read my own code than theirs.

I am aware of basic things like braces placement, consistency and indenting.

Are there any guidelines (like use of whitespaces) other than these that I may follow to improve upon this aspect?

thanks,

[LB]

Everyone has their own style that is the most effective for them to write and read. Pick a style you like, and try to follow it.

Could you give an example of some of your code that is hard to read and some of other people's code that you find easy to read? This will help us to figure out which style you would find most effective for you.

[abhishekm71]

Actually I wanted to ask whether there are any general rules that I can follow? sorry for being vague..

A sample code that I wrote is as follows. Note that I have tried to copy someone here by giving spaces after opening bracket and before closing bracket.

template <class T>
void mySort( const T& it_begin, const T& it_end ) {
    T rev_it = it_end;
    for ( T it1 = it_begin; ++it1 != it_end; ) {
        T it2 = it_begin;
        T temp_it = it_begin;
        for ( ; it2 != rev_it ; ) {
            temp_it = it2;
            if ( ++it2 != rev_it )
                if ( *temp_it > *( it2 ) )
                    std::iter_swap( temp_it, it2 );
        }
        rev_it = temp_it;
    }
}

and a code by an experienced programmer:

#include <iostream>
#include <functional>


int main()
{
	std::function<void( unsigned int )> reverse = [&]( unsigned int x ) 
	{ 
		std::cout << x % 10;
		( x /= 10 ) ? reverse( x ) : void( std::cout << std::endl );
	};


	while ( unsigned int x = ( std::cout << "\nEnter a non-negative number (0 - exit): ",
	                           std::cin >> x, x ) )
	{
		reverse( x );
	}

	return 0;

[1yR4jE8b]

That while loop is an abomination, please don't ever right anything remotely similar to that.

[cire]

That while loop is an abomination

How so?

With regard to mySort, I don't see any reason to pass the iterators by const reference. Definitely some whitespace could be used to make it more readable. More important than the whitespace, though, is reasonable names. I probably would've written it like so:

// for use with forward iterators
template <typename iter_type>
void my_sort(iter_type beg, iter_type end)
{
    const iter_type second(++iter_type(beg));

    while (second != end)
    {
        iter_type previous = beg;
        for (iter_type current = second; current != end; ++current)
        {
            if (*previous > *current)
                std::iter_swap(current, previous);
            previous = current;
        }

        end = previous;
    }
}

[Catfish4]

I would use even more empty lines, differentiating between declarations/definitions and function calls. So here's abhishekm71 original code indented with my own style:

template <class T>
void mySort(const T &it_begin, const T &it_end)
{
    T rev_it = it_end;

    for (T it1 = it_begin; ++it1 != it_end;)
    {
        T it2 = it_begin;
        T temp_it = it_begin;

        for (; it2 != rev_it ;)
        {
            temp_it = it2;

            if (++it2 != rev_it)
                if (*temp_it > *it2)
                    std::iter_swap(temp_it, it2);
        }

        rev_it = temp_it;
    }
}

Vertical space is wasted but readability is increased (in my opinion).

[N36fSL3A]

Don't forget commenting. Whenever I add anything to my game's core engine I comment as much as possible.

[Disch]

General tips:

1) Be consistent and correct with your indentation. This is absolutely the single most important thing ever when it comes to code readability.

2) Don't be afraid of whitespace. Adding a blank line here and there to separate 'ideas' in your code is very helpful. Much like how you'd make a separate paragraph in English text.

3) Try to keep functions small and with a singular purpose. If a function tries to do too many things it becomes difficult to follow. If doing multiple things in a function is unavoidable (or is impractical), then at least separate the concepts with whitespace and comments.

4) Write "good" comments. By "good" I mean a comment should not describe a single line of code. The comment should describe the overall process of what you're doing (only if it's not obvious) and/or why you're doing it. Example of a bad comment:

int foo = 5;  // assign 5 to foo 

It's obvious to anyone that that's what that code is doing, so that comment is worthless.

5) Try to make the code "self-documenting". That is.. function names reflect what the function actually does. Variable names reflect what the variable represents, etc.

6) Avoid the comma operator.

7) Don't try to cram too much on too few lines. Compact code is often difficult to read code. Spread it out. UNLESS there are a bunch of one-liners that are straightforward... in which case cramming them together with pretty print (vertically aligned) is acceptable... and arguably even preferred. Example:

class Clock
{
//...
    inline bool operator <  (time rhs) const        { return timestamp <  rhs;  }
    inline bool operator >  (time rhs) const        { return timestamp >  rhs;  }
    inline bool operator <= (time rhs) const        { return timestamp <= rhs;  }
    inline bool operator >= (time rhs) const        { return timestamp >= rhs;  }
    inline bool operator == (time rhs) const        { return timestamp == rhs;  }
    inline bool operator != (time rhs) const        { return timestamp != rhs;  }
//...
};

That's about all I can think of as far as rules go. Really the best thing you can do is just look at your code and figure out whether or not it's hard to read. If it is, clean it up so it's easier to read.

[ne555]

unsigned int x = ( std::cout << "\nEnter a non-negative number (0 - exit): ", std::cin >> x, x )
¿what's the value of `x' if the reading fails?
1) 0
a) garbage
\alpha) implementation defined

[1yR4jE8b]

+1 ne55

It's not a big problem in such a trivial program but it's still ugly, hard to follow, and just plain bad practice.

A weird thing about the way the operator,() works too, is that it "discards" the left hand side. So by following that, when I interpret the code it reads as

unsigned int x = x;

in my brain, which is weird in and off itself -- but wait! -- we had side-effects from the cin>> x which got "discarded"...ugghg.

The real problem is when you are trying to debug something like that when x is not a POD type, with overloaded operator>>() and operator,() (bonus points if they can throw). Changing the states of objects during construction -- using cin in this case -- (but not in the actual constructor) is just craziness. Writing code like this when it might not necessarily cause a problem will come back and bite you in the ass the second you try to write something non-trivial.

Also,

std::function<void( unsigned int )> reverse = [&]( unsigned int x ) 
{ 
    std::cout << x % 10;
    ( x /= 10 ) ? reverse( x ) : void( std::cout << std::endl );
};

Gives a warning with Clang, correct code for a recursive lambda function (due to quirks in type capture) is:

std::function<void( unsigned int )> reverse; 
reverse = [&reverse]( unsigned int x ) 
{ 
    std::cout << x % 10;
    ( x /= 10 ) ? reverse( x ) : void( std::cout << std::endl );
};

The original code is making a recursive call to the std::function variable when it isn't completely initialized yet.

And if I may offer a slight improvement (IMO) on Disch's code:

#include <utility>
using namespace std::rel_ops;

class Clock
{
//...
    inline bool operator <  (time rhs) const        { return timestamp <  rhs;  }
    inline bool operator == (time rhs) const        { return timestamp == rhs;  }
//...
};

When you use the rel_ops namespace, anything that has operator< and == defined get's the other reliational operators for free as they can all be expressed as combinations of these two.

[LB]

Disch wrote:

6) Avoid the comma operator.

There is a case where I disagree:

T *t = new T;
//...
delete t, t = nullptr;

//or

T *t = c_lib_class_new();
//...
c_lib_class_delete(t), t = nullptr;

[Disch]

RAII. Those situations should literally never happen.

Furthermore, the comma operator functions the same as a semicolon in that case:

delete t, t = nullptr;
//vs
delete t; t = nullptr;

[chrisname]

I don't think using the comma operator is a good idea in general. Nor is putting two semi-colon delimited statements on a single line, come to think of it.

I've only ever used the comma operator to do something like this:

#define return(x) __backtrace_append__(__func__), (x)

// ...
return(0);

And I didn't end up using it in the end anyway.

[1yR4jE8b]

All this talk about the comma operator is bringing back lots of bad memories...I'm so glad I work in a Rails shop now. Say what you will about the "black magic" in Rails/Ruby, it's nothing compared to what I used to see at the Qt shop I worked at before.

*curls up into fetal position*

[chrisname]

It may be hard for you, but reliving your trauma in a safe setting can make you feel better. We're here to help you. That being said: story time!

[vlad from moscow]

First of all never use this style of placing braces

void mySort( const T& it_begin, const T& it_end ) {

Instead use

void mySort( const T& it_begin, const T& it_end )
{

The first style is used by idiots that understand nothing in code writing.

Secondly do not use this style of declaring pointers or references as

int* p;

Use instead

int *p;

Again the first style are used by idiots who undersatnd nothing in code writing.

Thirdly after and before braces insert a space. For example

if ( SomeExpression )

is much better than

if (SomeExpression)

You should understand that an empty line is very important element of code formating. Try to seperate logical parts of the program with blank lines. For example let consider your function.

template <class T>
void mySort( const T& it_begin, const T& it_end ) {
    T rev_it = it_end;
    for ( T it1 = it_begin; ++it1 != it_end; ) {
        T it2 = it_begin;
        T temp_it = it_begin;
        for ( ; it2 != rev_it ; ) {
            temp_it = it2;
            if ( ++it2 != rev_it )
                if ( *temp_it > *( it2 ) )
                    std::iter_swap( temp_it, it2 );
        }
        rev_it = temp_it;
    }
}

It would look much better if there would be blank lines

template <class T>

void mySort( const T &it_begin, const T &it_end ) 
{
    T rev_it = it_end;

    for ( T it1 = it_begin; ++it1 != it_end; ) 
    {
        T it2 = it_begin;
        T temp_it = it_begin;

        while ( it2 != rev_it ) 
        {
            temp_it = it2;

            if ( ++it2 != rev_it )
            {
                if ( *temp_it > *( it2 ) ) std::iter_swap( temp_it, it2 );
            }
        }

        rev_it = temp_it;
    }
}

The problem is that some idiots that have no own intellect usually simply copy the style after another idiots that are well-known. You should critically consider all advices of so-called authorities.

[LB]

Disch wrote:

RAII. Those situations should literally never happen.

So you don't write constructors or destructors?

@vlad: I agree with you, why would someone report your post?

[1yR4jE8b]

Again the first style are used by idiots who undersatnd nothing in code writing.

I guess Linus Torvalds is an "idiot that knows nothing about code writing then"?

AMIRITE GUIZE?!

[Disch]

So you don't write constructors or destructors?

You don't delete things in ctors.
And you don't [need to] null things in dtors.

But generally I don't call delete directly, no. I use smart pointers whenever I need anything dynamically allocated, And for anything that's a C lib which has some other kind of interface for creation/destruction, I create [at least] a minimalistic RAII wrapper around it.

@vlad: I agree with you, why would someone report your post?

I didn't report it, but probably because he's being an asshole again and calling anyone who doesn't share his personal preference for formatting code an idiot in bold letters repeatedly.

[vlad from moscow]

Programmers as other people have their own weaknesses and caprices. Idiots repeat their caprices. Why? Because they are idiots. For exmaple many people smoked Malborro because their favorite actot also smoked Malborro. But as far as I know he died from cancer ( if I correctly have named the sickness).
Why do you have decided that Torvalds is good in writing code?!!! It seems that you are that idiot that blindly trust authorities. Tomorrow there will be another authority and you will refer to his stupidy.