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

Create a Christmas Tree

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

Problem

This function draws a Christmas tree (with an angel on top) with text (AKA, ASCII art).

#define ARRSZ(x) (sizeof(x)/sizeof(x[0]))

void drawChristmasTree (int height) {
    const char * const angel[] = { " ._.", "(\\o/)", ")/|\\(", "/_*_\\", };
    const char stars[] = "***************************************";
    int i;
    int offset = 20 - height;

    if ((height  ARRSZ(stars)/2)) {
        printf("The height has to be at least %zu and no more than %zu.\n",
               ARRSZ(angel), ARRSZ(stars)/2);
        return;
    }

    for (i = 0; i < height; ++i) {
        printf("%*s%s\n",
               height - (i < ARRSZ(angel) ? ARRSZ(angel) : i + 1), "",
               (i < ARRSZ(angel)) ? angel[i] : &stars[ARRSZ(stars) - 2*i]);
    }
    printf("%*s%c\n", height - 2, "", '*');
    printf("%*s%c\n", height - 2, "", '*');
}


For size 10, this code produces:

._.
(\o/)
)/|\(
/_*_\
***
*****
*
***
*
***
*
*

Solution

Compiler warnings

Always compile with warnings turned on. Your compiler should have warned you that offset is an unused variable. My compiler also complained about passing an unsigned int as an argument to printf("%*") with height - (i < ARRSZ(angel) ? ARRSZ(angel) : i + 1).

Error handling

The output should either be a Christmas tree or nothing at all. Printing an error message in place of a tree is just mean.

If you must print an error message, print it to stderr!

Code formatting

Why not make the angel look like an angel in your code? Granted, the double backslashes interfere with the alignment, but at least you could try.

const char * const angel[] = { " ._.",
                              "(\\o/)",
                               ")/|\\(",
                               "/_*_\\", };


Looping

With those conditional expressions within the loop, you might as well split it into two loops:

for (i = 0; i < ARRSZ(angel); ++i) {
    printf("%*s%s\n", height - (int)ARRSZ(angel), "", angel[i]);
}
for (; i < height; ++i) {
    printf("%*s%s\n", height - i - 1, "", stars + ARRSZ(stars) - 2*i);
}


Here, I've also rewritten &stars[ARRSZ(stars) - 2*i] using the more idiomatic pointer arithmetic.

Alternate solution

If you ensure that each line of the angel has the same length, then you could also write the printf() calls as

const char * const angel[] = { " ._. ",
                              "(\\o/)",
                               ")/|\\(",
                               "/_*_\\", };

for (i = 0; i < ARRSZ(angel); ++i) {
    printf(" %*s\n", (int)(height - ARRSZ(angel) + ARRSZ(angel[0]) / 2), angel[i]);
}
for (; i < height; ++i) {
    printf("%*.*s\n", height + i - 2, 2 * i - 1, stars);
}

Code Snippets

const char * const angel[] = { " ._.",
                              "(\\o/)",
                               ")/|\\(",
                               "/_*_\\", };
for (i = 0; i < ARRSZ(angel); ++i) {
    printf("%*s%s\n", height - (int)ARRSZ(angel), "", angel[i]);
}
for (; i < height; ++i) {
    printf("%*s%s\n", height - i - 1, "", stars + ARRSZ(stars) - 2*i);
}
const char * const angel[] = { " ._. ",
                              "(\\o/)",
                               ")/|\\(",
                               "/_*_\\", };

for (i = 0; i < ARRSZ(angel); ++i) {
    printf(" %*s\n", (int)(height - ARRSZ(angel) + ARRSZ(angel[0]) / 2), angel[i]);
}
for (; i < height; ++i) {
    printf("%*.*s\n", height + i - 2, 2 * i - 1, stars);
}

Context

StackExchange Code Review Q#73860, answer score: 3

Revisions (0)

No revisions yet.