Can this code be improved/refactored/optimised/compacted anymore? I've exhausted my creative juices and looking for your wisdom

David Wyatt 0
Viewed 43

This is my code for the CS50 Mario (more comfortable). Answers can be found all over the net however I wrote this myself.

I would like to refactor to its maximum potential (just for my own improvement I'm not enrolled or anything).

I have chosen to create 2 functions and called them in the main function.

Can this be be improved or refactored anymore or is this as good as it gets?

A quick outline of the code:

Firstly it asks for a valid input of an amount of rows to be built (an integer between 0 and 550) and then builds a Mario wall like this: # #

#include <stdio.h>
#include <stdlib.h>

void build_wall(int num_rows);
void get_rows(int n);
int rows = 0;
int spaces = 0;
int bricks = 0;
int main(void)

{
    get_rows(rows);
    build_wall(rows);
}
void get_rows(int n)
{
    do
    {
    printf("How many rows would you like? ");
    scanf("%d", &rows);
    printf("\n");
    printf("You would like %d rows.\n", rows);
    }
    while (rows <= 0 || rows >=550);
}

void build_wall(int num_rows)
{
    for (int i = 0; i < num_rows; i++) ///number of rows
    {
        for(spaces = 0; spaces < num_rows-i-1; spaces++ )       // first spaces
            {
                printf(" ");
            }

        for(bricks = 0; bricks < i+1; bricks++)             //   first wall bricks
            {
                printf("#");
            }

        {
            printf("  ");                                   //Middle spaces
        }

        for(bricks = 0; bricks < i+1; bricks++)            // Second wall bricks
            {
                printf("#");
            }
    printf("\n");
    }

}
cs50 c
1 Answers

You asked, so here is an answer. Factor out the several loops and a couple of printf()s would be a start. And make better use of 'i' (renamed to 'r' as in 'row_counter'.)

void out( char c, int n ) {
    while( n-- )
        putchar( c );
}

void build_wall( int nrows ) {
    for ( int r = 1; r <= nrows; r++ ) {
        out( ' ', nrows - r ); // left indent
        out( '#', r ); // left wall
        out( ' ', 2 ); // 'chimney'
        out( '#', r ); // right wall
        out( '\n', 1 ); // done with this row
    }
}

EDIT: Since OP reports problems (misunderstanding?) integrating the above function into the original code, why bother with functions at all. This is a single purpose program

#include <stdio.h>

int main() {
    int rows = 0, c, n;
    do {
        printf( "How many rows would you like? (2-30)" );
        scanf( "%d", &rows );
        printf( "\nYou would like %d rows.\n", rows );
    } while( rows < 2 || 30 < rows ); // be reasonable

    for ( int r = 1; r <= rows; r++ )
        for( int step = 0; step < 5; step++ ) {
            switch( step ) {
                case 0: c = ' '; n = rows - r; break;
                case 1: c = '#'; n = r; break;
                case 2: c = ' '; n = 2; break; // 'chimney'
                case 3: c = '#'; n = r; break;
                case 4: c = '\n'; n = 1; break;
            }
            while( n-- )
                putchar( c );
        }
    return 0;
}

EDIT2: In fact, while we are at it, replace the inner for(), the switch(), and the while(). printf() offers the facilities needed. For convenience, an infinite loop makes it easy to play multiple times.

#include <stdio.h>

int main() {
    for( ;; ) {
        int rows = 0;
        do {
            printf( "How many rows would you like? (2-30)" );
            scanf( "%d", &rows );
            printf( "\nYou would like %d rows.\n", rows );
        } while( rows < 2 || 30 < rows ); // be reasonable, eh?

        char *x = "                              "
                  "##############################";

        for ( int r = 1; r <= rows; r++ )
            printf( "%.*s  %.*s\n", rows, x+30-rows+r, r, x+30 );
    }
    return 0;
}
Fe2O30
Related