HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppModerate

Printing a pyramid

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
printingpyramidstackoverflow

Problem

Can you please review this code to determine if it's good or can be improved?

#include "stdafx.h"
#include 
#include 

using namespace std;

int _tmain(int argc, _TCHAR* argv[])
{

    int h = 20;
    int bl = 1;

    while (h != 0) {
        for (int i = 0; i != h; i++) {
            cout << " ";
        }
        for (int i = 0; i != bl; i++) {
            cout << "*";
        }
        cout << endl;
        h = h - 1;
        bl = bl + 2;
    }
    return 0;
}

Solution

First, it isn't too good of an idea to use using namespace std; as discussed here. There are times and places to use it, but you should know about this.

Second, it is obvious you are using VC++ (which isn't bad, but isn't exactly the same as C++). You should write your main like this instead:

int main()


I'm not sure what the h and bl names are supposed to mean (height and baseline?), use descriptive names.

This can be a for loop:

while (h != 0) {


Write it like this:

for (int height = 20; height > 0; --height) {
    for (int i = 0; i != height; i++) {
        cout << " ";
    }
    for (int i = 0; i != bl; i++) {
        cout << "*";
    }
    cout << endl;
    bl = bl + 2;
}


In fact, you can even put the bl variable in the loop too:

for (int height = 20, baseline = 1; height > 0; --height, baseline += 2) {
    for (int i = 0; i != height; i++) {
        cout << " ";
    }
    for (int i = 0; i != baseline; i++) {
        cout << "*";
    }
    cout << endl;
}


Use the prefix ++ and -- operators whenever possible because the postfix operators have worse performance due to the need for them to create a copy of the object to return before incrementing the value.

for (int i = 0; i < baseline; ++i) {


Also, in a loop like this, you typically do i < baseline, not i != baseline.

If you want to remove the inner loops entirely, you can rewrite the entire program to just this:

for (int height = 20, baseline = 1; height > 0; --height, baseline += 2) {
    std::cout << std::string(height, ' ')
              << std::string(baseline, '*')
              << std::endl;
}

Code Snippets

while (h != 0) {
for (int height = 20; height > 0; --height) {
    for (int i = 0; i != height; i++) {
        cout << " ";
    }
    for (int i = 0; i != bl; i++) {
        cout << "*";
    }
    cout << endl;
    bl = bl + 2;
}
for (int height = 20, baseline = 1; height > 0; --height, baseline += 2) {
    for (int i = 0; i != height; i++) {
        cout << " ";
    }
    for (int i = 0; i != baseline; i++) {
        cout << "*";
    }
    cout << endl;
}
for (int i = 0; i < baseline; ++i) {
for (int height = 20, baseline = 1; height > 0; --height, baseline += 2) {
    std::cout << std::string(height, ' ')
              << std::string(baseline, '*')
              << std::endl;
}

Context

StackExchange Code Review Q#87454, answer score: 12

Revisions (0)

No revisions yet.