Skip to main content
Code Review

Return to Answer

edited body
Source Link
200_success
  • 145.5k
  • 22
  • 190
  • 478

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 rn - 1, because the rnth row contains rn entries.

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 r - 1, because the rth row contains r entries.

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.

discuss inner loop bounds
Source Link
200_success
  • 145.5k
  • 22
  • 190
  • 478

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 r - 1, because the rth row contains r 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).

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).

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 r - 1, because the rth row contains r 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).

Source Link
200_success
  • 145.5k
  • 22
  • 190
  • 478

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.

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.
lang-cpp

AltStyle によって変換されたページ (->オリジナル) /