Generating formatted multiplication tables in C++

Question

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 maxcol^th position print a newline.

#include<iostream>
#include<iomanip>    // For setw()
#include<conio.h>    // For getch()
using namespace std;
int main()
{
    int number;
    cout << "Enter the number whose table you want: ";
    cin >> number;
    int maxrows; int maxcols;
    cout << "\nEnter the number of rows you want: "; cin >> maxrows;
    cout << "\nEnter the number of columns : "; 
    cin >> 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 should be redundant as we need to increment only pos to find the value at that position. There must be some way to tell the computer to print a newline after every maxcol number of executions -- this is acheived by the if statement.

#include<iostream>
#include<iomanip>
#include<conio.h>
using namespace std;
int main()
{
    int number;
    cout << "Enter the number whose table you want: ";
    cin >> number;
    int maxrows; int maxcols;
    cout << "\nEnter the number of rows you want: "; cin >> maxrows;
    cout << "\nEnter the number of columns : "; cin >> maxcols;
    int row; int col; //Current row and current col

    for(int pos=1; pos<=(maxrows*maxcols); pos++)
    {
        cout << setw(6) <<number*pos << "  ";
        if((pos%maxcols)==0)
        {
          cout << "\n"; 
        }

    }
    getch();    
}

The problem is the if statement doesn't get executed most of the times. For 10 maxcols if gets executed only 10% times. It mostly wastes cpu time. In the previous version every statement does some important function every single time it is executed.

Keep track of current row and column: The idea was to locate the current position by knowing the current row and column of that position. It is quite similar to the first solution but there is an important advantage that I'll mention after the code.

#include<iostream>
#include<iomanip>
#include<conio.h>
using namespace std;
int main()
{
    int number;
    cout << "Enter the number whose table you want: ";
    cin >> number;
    int maxrows; int maxcols;
    cout << "\nEnter the number of rows you want: "; cin >> maxrows;
    cout << "\n Enter the number of columns : "; cin >> maxcols;

    for(int row=0; row<=(maxrows-1); row++)
    {
        for(int col=1; col<=maxcols; col++)
        {
            cout << setw(6) << number*(col + row*maxcols); // (col + row*maxcols) represent the current position
        }
        cout << "\n";
    }
    getch();
}

Variable row is not the current row but number of rows above the current row. row*maxcols is the position of first number of current row. col is simply the current column. The beauty of this approach is that the way current position is determined allows us to print the multiplication table in vertically increasing manner too. As per the exercise the numbers 7, 14, 21 ... are printed horizontally -- what if I wanted to print them vertically and jump to next column after a particular number of rows? The way console window is designed we can't print above the current printed line -- we can't go on printing 7, newline , 14, newline..., then jump to next column and print at the initial top row. Here is the solution:

#include<iostream>
#include<iomanip>    // For setw()
#include<conio.h>    // For getch()
using namespace std;
int main()
{
    int number;
    cout << "Enter the number whose table you want: ";
    cin >> number;
    int maxrows; int maxcols;
    cout << "\nEnter the number of rows you want: "; cin >> maxrows;
    cout << "\n Enter the number of columns : "; 
    cin >> maxcols;



    for(int row=1; row<=maxrows; row++ )
    {       
        cout << "\n";
        for(int col=0; col<maxcols; col++)
        {
            cout << setw(6)<< number*(row + maxrows*col) << "  ";
        }   


    }
    getch();    
}

row is the current row now and col is the number of columns to the left of current position. (row + maxrows*col) represent the current position.

---

Question: - Please study every code individually and tell if the idea used could be implemented better. Could the number of variables used deceased further? Could the algorithm be improved further? Could the code be more efficient/optimised further?

• In the second version I removed one loop and instead used an if statement? Which is better? Use two loops or one loop plus an if statement?

• Overall which version is more professional/good/efficient? Which one would be faster at run time?

• Bonus optional question: I am new to programming and this is the first book which I've started reading seriously. Is the way I am solving exercises ok for a starter? I'm not asking for opinion based answer. Just a suggestion.

Answer 1

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 <conio.h> 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 <iostream>
#include <iomanip>

int main()
{
    int number;
    std::cout << "Enter the number whose table you want: ";
    std::cin >> number;
    unsigned maxrows; 
    unsigned maxcols;
    std::cout << "\nEnter the number of rows you want: "; 
    std::cin >> maxrows;
    std::cout << "\nEnter the number of columns : "; 
    std::cin >> 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

Answer 2

No of times condition gets checked:

• In all your examples except second one, condition was checked maxrows*maxcols times.

• In second example, checking is done 2*maxrows*maxcols times. While this is not a significant drawback, it certainly is a bit inefficient.

Possible Improvements:

• As highlighted by other answers, strive for readable code. Optimisation is very important, but mainly if it causes significant changes to efficiency. Its better to put our concentration in improving algorithm and the big picture, rather than keep on tinkering with tiny details. That being said, adopting good habits, which provide us with legible and fairly efficient code, is in my opinion the way to go. Micro-optimisations often lead to cryptic code.

• Use type-modifiers like unsigned where-ever applicable. For example, the iteration variables are positive, so change their type to unsigned int.
  Perhaps you should not use unsigned for sum and num variables (refer code below), because number entered could be negative.

• Lastly, although it doesn't make any significant changes, but addition is done more efficiently in computers than multiplication, so you can use repeated addition instead of multiplying everytime (note the sum += num) :

  #include <iostream>
  #include <iomanip>
  
  int main() {
  
      int num;
      std::cout << "Enter no: ";
      std::cin >> num;
  
      unsigned int row = 20, col = 10;
  
      int sum = 0;
      num < 0     
  
      for( unsigned int i = 0; i < row; ++i) {
  
          for( unsigned int j = 0; j < col; ++j) {
              sum += num;
              std::cout << std::setw(6) << sum;
          }
          std::cout << '\n';
      }
  
      return 0;
  }
  

Use standard C++

• Instead of using non-standard getch() from conio.h, use cin.get() from iostream

• Do not use using namespace std;. Refer link.
  If you want to use cin and cout, prepend them with their namespace, followed by scope resolution operator:
  std::cout << " "; and std::cin >> x;

Answer 3

In a sense, you have already reviewed your own code. You have weighed the pros and cons of various approaches, and even challenged yourself to consider how to transpose the output. It's no surprise that your question has been well received on Code Review.

Be careful to look at the big picture. None of these solutions is bad code, even if some of these solutions are better than others. Even when we say that something is "inefficient", it's just a matter of craftsmanship rather than a real performance issue, because any computation will be dwarfed by the I/O and program initialization overhead. Producing maintainable code is more important than saving a few CPU cycles.

---

The most idiomatic way to do something n times in C/C++ is

for (int i = 0; i < n; i++) {
    …
}

It can also be done with for (int i = 1; i <= n; i++), but only if you have a good reason to start at 1. A third way is while (n--) { … }, if you don't need to preserve the original value of n and don't need to count upwards. Any other loop structure, such as

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

… would be unconventional and likely to lead to off-by-one errors.

It's best to avoid recomputing invariants such as maxrows * maxcols in the loop condition.

---

I'm not a fan of the first two approaches.

The first approach has pos initialized on one line, tested on a second line, and incremented in a third place. That makes it hard to follow.

The second approach tries to perform a two-dimensional task using one linear loop. I have some mild concerns about computing maxrows*maxcols and (pos%maxcols)==0 every time you print a number. You also left the declarations int row; int col; for unused variables there; compiling with warnings enabled should have warned you about that.

The third approach, with two nested for-loops, is the most straightforward way to make a grid. However, the implementation is marred by the unconventional loop counting, as mentioned above.

I think that the most elegant solution would be to decouple the grid-counting loops from the content-generating calculation:

int n = 0;
for (int row = 0; row < maxrows; row++) {
    for (int col = 0; col < maxcols; col++) {
        std::cout << std::setw(7) << (n += multiple);
    }
    std::cout << "\n";
}

You can also use that technique to produce a transposed table, though it's less elegant.

int stride = maxrows * multiple;
for (int row = 1; row <= maxrows; row++) {
    for (int col = 1, n = row * multiple; col <= maxcols; col++, n += stride) {
        std::cout << std::setw(7) << n;
    }
    std::cout << "\n";
}

Answer 4

There is an excellent way to print tables using the conditional (ternary) operator.

For example, to print a weekly calendar, the C version is:

date=1 ; while(date <= monthLength)
         printf("%2d%c",date++,(date-offset) % 7 ? ' ' : '\n');

It works by appending a linefeed when (date-offset)%7 is zero, otherwise a space between table columns.

offset is chosen so the weekends are right for each month.