patterncppModerate
Printing out a diamond with a user-inputted size
Viewed 0 times
inputtedwithdiamondusersizeprintingout
Problem
I made a program that prints out a diamond with
n lines. n is equal to whatever the user inputted. I know I overuse the ternary or conditional operator, but in my opinion, it makes the code more compact, and since I am the only one reading my code, readability doesn't matter to me. My friend challenged me to write one of these programs in under 50 lines of code, so that is why I am cramming everything together.#include
int main() {
int lineNum = 1, lines, stars = 1, spaces = 0;
std::cin >> lines;
// if lines is even, split it equally, if not, split in half and add 1
while (lineNum <= ((lines % 2 == 0) ? (lines / 2) : (lines / 2) + 1)) {
for (int c = (((lines % 2 == 0) ? (lines / 2) : (lines / 2) + 1) - lineNum); c != 0; c--)
std::cout << " ";
for (int c = 0; c < stars; c++)
std::cout << "*";
std::cout << "\n";
lineNum++;
stars += 2;
}
stars = (lines % 2 == 0) ? stars - 2 : stars - 4;
spaces = (lines % 2 == 0) ? spaces = 0 : spaces = 1;
while (lineNum <= lines) {
for (int c = 0; c < spaces; c++)
std::cout << " ";
for (int c = stars; c != 0; c--)
std::cout << "*";
std::cout << "\n";
stars -= 2;
spaces++;
lineNum++;
}
}Solution
I see a number of things that could help you improve your code.
Don't disregard readability
Readability of code is always of use and always of interest, even if you're the only one who ever sees the code. Since, by definition, you're asking for a code review by posting here, even that assumption is clearly faulty, because every reviewer is going to have to read and understand your code to comment usefully on it. Even if you really were the only person to ever look at the code, if you decide to modify it some time later, readability will be important to you.
Decompose your program into functions
All of the logic here is in
Don't further clutter trinary operators
The code currently includes this line:
The problem is that the
Simpler, and better in my view, would be this:
Rethink your algorithm
The biggest improvements are often found in simply rethinking and reimplementing the basic algorithm. In this case, each line is 0 or more spaces, followed by 1 or more stars. For each increasing line in the first half, the number of spaces decrements by one and the number of stars increments by two. A little bit of examination with paper and pencil will quickly yield algebraic expressions for the number of spaces and number of stars for each line in the top half of an n-line diamond pattern. The bottom half is simply the reverse of the top half. This suggests a simple algorithm.
Putting it all together
Your original program, as posted, was 44 lines long. Using all of these hints, and with better readability, here it is in exactly half that many lines:
Don't disregard readability
Readability of code is always of use and always of interest, even if you're the only one who ever sees the code. Since, by definition, you're asking for a code review by posting here, even that assumption is clearly faulty, because every reviewer is going to have to read and understand your code to comment usefully on it. Even if you really were the only person to ever look at the code, if you decide to modify it some time later, readability will be important to you.
Decompose your program into functions
All of the logic here is in
main in one rather long and dense chunk of code. It would be better to decompose this into separate functions.Don't further clutter trinary operators
The code currently includes this line:
spaces = (lines % 2 == 0) ? spaces = 0 : spaces = 1;The problem is that the
spaces = 0 and spaces = 1 on the right side are wholly uneccesary. If you must use trinary operators, (and I question that in this case), use them correctly:spaces = (lines % 2 == 0) ? 0 : 1;Simpler, and better in my view, would be this:
spaces = lines & 1;Rethink your algorithm
The biggest improvements are often found in simply rethinking and reimplementing the basic algorithm. In this case, each line is 0 or more spaces, followed by 1 or more stars. For each increasing line in the first half, the number of spaces decrements by one and the number of stars increments by two. A little bit of examination with paper and pencil will quickly yield algebraic expressions for the number of spaces and number of stars for each line in the top half of an n-line diamond pattern. The bottom half is simply the reverse of the top half. This suggests a simple algorithm.
Putting it all together
Your original program, as posted, was 44 lines long. Using all of these hints, and with better readability, here it is in exactly half that many lines:
#include
void multiprint(unsigned n, char ch) {
for ( ; n; --n)
std::cout half) n = maxline - n - 1;
multiprint(half - n, ' ');
multiprint(n*2 + 1, '*');
std::cout > lines;
for (int i =0; i < lines; ++i)
line(i, lines);
}Code Snippets
spaces = (lines % 2 == 0) ? spaces = 0 : spaces = 1;spaces = (lines % 2 == 0) ? 0 : 1;spaces = lines & 1;#include <iostream>
void multiprint(unsigned n, char ch) {
for ( ; n; --n)
std::cout << ch;
}
void line(unsigned n, unsigned maxline) {
unsigned half = (maxline-1)/2;
if (n > half) n = maxline - n - 1;
multiprint(half - n, ' ');
multiprint(n*2 + 1, '*');
std::cout << '\n';
}
int main() {
int lines;
std::cin >> lines;
for (int i =0; i < lines; ++i)
line(i, lines);
}Context
StackExchange Code Review Q#106307, answer score: 10
Revisions (0)
No revisions yet.