Generating formatted multiplication tables in C++

I am self studying C++ from Robert Lafore's OOP in C++. I am solving the first exercise questions of chapter-3 loop and decisions. I've written three different codes using different approaches. I want to get feedback over my codes like a teacher is supposed to give feedback to his student. I want to get feedback over each code individually and then want to know which solution is overall better or more efficient.

The exercise is:


Assume that you want to generate a table of multiples of any given number. Write a program that allows the user to enter the number and then generates the table, formatting it
into 10 columns and 20 lines. Interaction with the program should look like this (only the
first three lines are shown):

Enter a number: 7
    7     14     21     28     35     42     49     56     63     70
   77     84     91     98    105    112    119    126    133    140
  147    154    161    168    175    182    189    196    203    210

Using a for loop inside while loop:
I've written solution for user-defined number of columns and rows. The most obvious solution was to keep track of current position in the table. The number at a particular position should original_num * pos. And after every maxcolth position print a newline.

#include
#include    // For setw()
#include    // For getch()
using namespace std;
int main()
{
    int number;
    cout > number;
    int maxrows; int maxcols;
    cout > maxrows;
    cout > maxcols;
    int pos=1; //starting position

    while( pos<=maxrows*maxcols )
    {
        for(int col=1; col<=maxcols; col++, pos++)
        {
            cout<< setw(6) << pos*number << "  ";
        }
        cout<<"\n";
    }
    getch();
}

The inner for loop finishes after maxcol number of repeatitions. The outer while loop puts a newline and repeats again.

Using a single for loop with an if statement inside: Another idea was to force myself to use only one loop because, in principle, the second loop sh

I see a number of things that could help you improve your program.

Don't abuse using namespace std

Putting using namespace std within your program is generally a bad habit that you'd do well to avoid.

Strive for portable code

Several of the features of this code are either platform-specific or compiler-specific or both. Specifically, #include and getch() are non-standard and not portable. Instead consider portable replacements. For instance you could use std::getchar() instead.

Use whitespace to improve readability

Lines like this:

for(int row=0; row<=(maxrows-1); row++)

become much easier to read with a little bit of whitespace:

for(int row = 0; row <= maxrows - 1; row++)

Prefer ++i to i++ in loops

There is not a big difference for most uses, but if you don't need to save the pre-incremented value, make it simple for both the reader and the compiler and say ++i. This also will help when you start using iterators which often only implement the prefix ++ operator.

Use language idioms

The classic C++ (and C) method of iteration through N things is like this this:

for(int i=0; i < N; ++i) { /* do something */ }

Note that counting starts from 0 and that the < operator is used. The current code (last variant) uses this instead:

for(int row=0; row<=(maxrows-1); row++) {
    for(int col=1; col<=maxcols; col++) {
        // do stuff
    }
}

Think about signed versus unsigned

What should the program do with a negative number for maxrows? I don't know and probably neither do you, since it doesn't really make sense. For that reason, it would make more sense for maxrows and maxcols to be declared as unsigned.

Think carefully about the problem

Each time the program prints, it's printing the next multiple of number. Rather than multiply each time, why not simply add? I'd write it like this:

int accum = number;
for(unsigned row = 0; row < maxrows; ++row) {
    for(unsigned col = 0; col < maxcols; ++col) {
        std::cout << std::setw(6) << accum;
        accum += number;
    }
    std::cout << "\n";
}

Don't optimize blindly

First, we should keep in mind what Donald Knuth has said about optimization:


The real problem is that programmers have spent far too much time worrying about efficiency in the wrong places and at the wrong times; premature optimization is the root of all evil (or at least most of it) in programming.

Your particular program probably spends 99% of its time doing I/O, so the performance differences among the variations you present is probably best approximated as zero.

We should keep in mind yet another famous quote from Knuth,


Let us change our traditional attitude to the construction of programs: Instead of imagining that our main task is to instruct a computer what to do, let us concentrate rather on explaining to human beings what we want a computer to do.

For that reason, it seems to me that the last version (with nested for loops) is probably better because it is easier for humans to understand the code. I'd write it as shown above.

Result

For the column-first version, we can still do it this way with constant adders each loop iteration. That would look like this:

#include 
#include 

int main()
{
    int number;
    std::cout > number;
    unsigned maxrows; 
    unsigned maxcols;
    std::cout > maxrows;
    std::cout > maxcols;

    const int colincr = number * maxrows;
    const int rowdecr = number * (maxrows * maxcols - 1);
    int accum = number;
    for(unsigned row = 0; row < maxrows; ++row) {
        for(unsigned col = 0; col < maxcols; ++col) {
            std::cout << std::setw(6) << accum;
            accum += colincr;
        }
        accum -= rowdecr;
        std::cout << '\n';
    }
}

Example:

Enter the number whose table you want: 7

Enter the number of rows you want: 5

Enter the number of columns : 8
     7    42    77   112   147   182   217   252
    14    49    84   119   154   189   224   259
    21    56    91   126   161   196   231   266
    28    63    98   133   168   203   238   273
    35    70   105   140   175   210   245   280

StackExchange Code Review Q#122058, answer score: 12