4
\$\begingroup\$

We were asked to make a triangular number pattern in C++ with minimum loops:

____1_____
___2__3____
__4__5__6__
7__8__9__10

Code:

#include <iostream>

using namespace std;
int main() {
    int n=0, r=0, i=0;
    cout << "No. of rows: ";
    cin >> r;

    for( n=1; n<=r; n++) {
        for( i=1; i<=r-n; i++) {
            cout << "  ";
        }
        for( i=(n*(n-1)/2)+1; i<=(n*(n+1)/2); i++ ) {
            if( i<10 )
                cout << " " << i << "  ";
            else
                cout << i << "  ";
        }
        cout << "\n";
    }
    return 0;
}

Output:

enter image description here

Here's what I'd like reviewed:

  1. Is it wise to use pattern-generating formulas? For example, for putting value of i in the last loop, I used the formula for the pattern \$1,2,4,7\$... as \$\frac{n*(n-1)}{2}+1\$. Is it efficient? This involves four operations in each iteration, and it's fairly slow.
  2. Is it possible to reduce the number of loops? Is it better to reduce variables or reduce loops?
\$\endgroup\$
  • 1
    \$\begingroup\$ what happens if you put in a value like 14? You only account for 1 and 2 digit numbers, but 14(15)/2 > 99, and thus will have 3 digit numbers in it \$\endgroup\$ – spyr03 Sep 7 '15 at 15:01
  • \$\begingroup\$ @spyr03 Yea, but that is not a problem, i can increase no of spaces in if condition, and add an else if for nos greater than 99. Thanks! \$\endgroup\$ – Max Payne Sep 7 '15 at 15:05
  • \$\begingroup\$ @spyr03 is it effieicnt to use generating formulas? it involves 4 operations everytime. cant it be minimised? \$\endgroup\$ – Max Payne Sep 7 '15 at 15:07
  • \$\begingroup\$ I've edited out your last question. We will review code for you, but we won't write new code for you, unless a reviewer wants to. \$\endgroup\$ – Ethan Bierlein Sep 7 '15 at 15:13
3
\$\begingroup\$

Spacing

Use as much whitespace as needed, it makes it easier to read code, especially formulas or expressions with a lot of operators.


using namespace std;

This is discouraged, I don't think it makes much difference in a program of this size, but it can be a bad habit to rely on.


int n=0, r=0, i=0;

Try and only declare variables in the smallest scope possible, as in right when you need them. Since n and i are only ever used inside the loops, only declare them there.

I would avoid assigning 0 to r, as then if later the program crashes, and I see r no value, it makes the bug much easier to spot than if r has a valid value.


for( n=1; n<=r; n++) {
    for( i=1; i<=r-n; i++) {
        cout << "  ";
    }

This is completely up to you, but I would start the loops at 0, and go until n < r or i < r-n. That is because I would consider that a standard loop, and it is very quick to see how many times it loops, what stopping condition is, and what it loops over. I understand why you are starting from 1, the number of rows, but since this is never actually used, an off by one error wouldn't actually be noticeable. It is barely worth thinking about, but if you are writing for loops over and over, it makes it easier to read. Take this with a grain of salt.


for( i=(n*(n-1)/2)+1; i<=(n*(n+1)/2); i++ ) {

As we are printing out the natural numbers, starting from 1, and going to r*(r+1)/2, I would just have a variable that keeps counting upwards. Then I don't have to work out what number the row starts with each time, and you are incrementing i anyway, so there is essentially no extra work done by having a count, and you save a few operations too.


    if( i<10 )
        cout << " " << i << "  ";
    else
        cout << i << "  ";
}

This is exactly what padding was made for. I don't really know how to pad with cout, but printf makes it very easy. It looks like:

printf("%*d in the bed, and the little one said %s", the_amount_of_spaces, the_number_to_print, "a_piece_of_inpsiring_text");

Where everything inside the quotes is what to print, % means the next piece is a variable, %d means an int, %s means a string

There are some other things you can do, like if you never want to change the number of spaces, you can hardcode it like: printf("%3d ", count);

Here is a reference for it


Below are all the changes I would make thrown together, if you have any questions, ask away

#include <iostream>
#include <stdio.h>

int main() {
    int r;
    std::cout << "No. of rows: ";
    std::cin >> r;

    int count = 1;
    for(int n = 1; n <= r; n++) {
        for(int i = 0; i < r - n; i++) {
            std::cout << "  ";
        }

        while(count <= (n*(n + 1)/2)) {
            printf("%3d ", count);
            count++;
        }
        std::cout << "\n";
    }
    return 0;
}

The padding works well up until 45 rows are entered, after that, it was too wide for my screen to properly read.

| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ Formatting with <iostream> can be done with <iomanip>, namely std::setw here as in std::cout << std::setw(3) << 1; will output two spaces and a one. \$\endgroup\$ – Mat Sep 7 '15 at 16:57
  • \$\begingroup\$ @Mat That is interesting, is there a way to affect all cout, and not just a local one? \$\endgroup\$ – spyr03 Sep 7 '15 at 18:17
  • \$\begingroup\$ That setw resets every time you output something. Most other settings persist I think. iostream and iomanip are not really trivial once you go past the basics. Fortunately you usually only need the basics :-) \$\endgroup\$ – Mat Sep 7 '15 at 18:30
  • \$\begingroup\$ First of all thanks for such a nice answer which gives me many good habits! I want to ask that how can i check the time taken to execute and memory used? \$\endgroup\$ – Max Payne Sep 8 '15 at 8:22
2
\$\begingroup\$

Your main() does two things: prompt for the triangle size, then print the triangle. It's worth defining a separate function for the work of actually printing out the triangle, parameterizing the number of rows and the output destination.

Your output only looks triangular if all entries are just one or two digits. Anything larger than that, and your triangle will look lopsided. Handling the general case is a bit tricky: you have to figure out what the largest entry is, then calculate how many digits it has — all before even printing the 1. To print every number with the same width, I suggest using std::setw() instead of outputting spaces yourself.

In the termination condition of your inner loop, the upper bound (n*(n+1)/2) is constant, and should be factored out of the loop. You could simplify that formula: the first entry (n*(n-1)/2)+1 and the last entry (n*(n+1)/2) differ by n - 1, because the nth row contains n entries.

The whole thing can be implemented without using any visible loops at all. You would be basically doing in C++. Functional program avoids mutation, and instead stores all of its state in the call stack. To drive home the point about no mutation, I've declared everything except the ostream as const (even though the const int parameters are overkill).

#include <iomanip>
#include <iostream>
#include <cmath>

class Triangle {
  public:
    const int rows;

    // entryWidth: The number of digits in the largest entry, plus one space
    // halfWidth: Half of the width of the entire triangle
    explicit Triangle(const int rows) :
        rows(rows),
        entryWidth(1 + std::ceil(std::log10(lastEntryForRow(rows) + 1))),
        halfWidth(rows * entryWidth / 2 - 1) {
    }

  private:
    const int entryWidth, halfWidth;
    friend std::ostream &operator<<(std::ostream &out, const Triangle &t);

    /** Last entry for row r (with rows numbered from 1) */
    static int lastEntryForRow(const int r) {
        return r * (r + 1) / 2;
    }

    /** Write row r and all subsequent rows */
    void writeRowsStartingFrom(std::ostream &out, const int r) const {
        const int firstEntry = lastEntryForRow(r - 1) + 1,
                  lastEntry  = lastEntryForRow(r);
        out << std::setw(halfWidth - entryWidth * r / 2 + entryWidth)
            << firstEntry;
        writeEntries(out, firstEntry + 1, lastEntry);
        if (r < rows) {
            writeRowsStartingFrom(out << '\n', r + 1);
        }
    }

    void writeEntries(std::ostream &out, const int first, const int last) const {
        if (first <= last) {
            out << std::setw(entryWidth) << first;
            writeEntries(out, first + 1, last);
        }
    }
};

std::ostream &operator<<(std::ostream &out, const Triangle &t) {
    if (t.rows) t.writeRowsStartingFrom(out, 1);
    return out;
}

int main(int argc, char *argv[]) {
    int r;
    std::cout >> "Number of rows: ";
    std::cin >> r;
    std::cout << Triangle(r) << std::endl;
}

Functional programming does have benefits, but C++ is not designed to be used with FP, and this solution certainly isn't idiomatic C++. It's likely to be a bit slower — not that you would notice for any triangle that you would ever want to print. It's also longer than your original code because:

  • I've created a Triangle class to make main() prettier.
  • It's more generalized than your original solution.
  • A lot of the code is concerned with width adjustments.
  • I've defined a helper function lastEntryForRow() to avoid the repetition of the n*(n+1)/2 formula and make its purpose clear.
  • Recursion involves more typing than looping.
| improve this answer | |
\$\endgroup\$
1
\$\begingroup\$

The issue is that we want to reduce the number of loops in the code. It can be done with one loop over numbers from 1 to r. Outline: create a string of spaces as long as the longest line (easy formula), `print' each number in the correct place (slightly tricky formula) in this string. When the end of a line is reached (easy formula), write the string out and re-blank it.

Also consider that this could be written as a recursive function, and then you don't have any `loops' at all!

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.