How to make this piece of C code better and more readable-CodePudding

I need to change this code to make it readable and in better coding style. Basically it prints a roof made of X's. The line that needs to be changed the most is marked with comment.

int center = width / 2;

for (int i = 0; i < center; i  )
{
    printf(" ");
}
printf("X\n");

for (int i = 1, j = center - 1, l = 1; i < center; i  , j--, l  = 2) // This line !
{
    for (int h = 0; h < j; h  )
    {
        printf(" ");
    }
    printf("X");

    for(int k = 0; k < l; k  )
    {
        printf(" ");
    }
    printf("X\n");
}

CodePudding user response：

This isn't more readable (sorry, maybe someone else?), but it does simplify that line completely. You can infer j and l rather than increment them.

  int width = 7;
  int center = width / 2;

  for (int i = 0; i < center; i  )
    {
      printf (" ");
    }
  printf ("X\n");

  for (int i = 1; i < center; i  )  // This line !
    {
      for (int h = 0; h < (center - i); h  )
        {
          printf (" ");
        }
      printf ("X");

      for (int k = 0; k < (((i - 1) * 2)   1); k  )
        {
          printf (" ");
        }
      printf ("X\n");
    }

I'm sure there are more improvements but I'll leave you to it :)

CodePudding user response：

Using variables with the right meaning Use a function to simplify the core part

#include "stdio.h"

static void printnspace(int nb) {
    for (int i = 0; i < nb; i  )
    {
        putchar(' ');
    }
}

int main(void) {
    int width = 11;
    int center = width / 2;
    int mid = 1;

    printnspace(center);
    printf("X\n");

    for (int left = center - 1; left > 0; left--) {
        printnspace(left);
        putchar('X');
        printnspace(mid);
        mid  = 2;
        printf("X\n");
    }
    return 0;
}

CodePudding user response：

You could drop l and replace the reference to this variable with (i - 1) * 2 1. Same with j, you can express it using center - i. Also, instead of printing " " n time, you could print one string of n " " a single time. I don't know if printf has such features, if not, you can just extract the logic into it's own function.

Edit: finally, you could clarify the meaning of center - i and (i - 1) * 2 1 by storing them in variables.

int center = width / 2;

void print_padded_tile(int pad_length) {
    for (int i = 0; i < pad_length; i  ) {
      printf(" ");
    }
    printf("x");
}

print_padded_tile(center);
printf("\n");

for (int i = 1; i < center; i  )
{
    int space_count_until_first_tile = center - i;
    print_padded_tile(space_count_until_first_tile);
    int space_count_until_second_tile = (i - 1) * 2   1;
    print_padded_tile(space_count_until_second_tile);
    printf("\n");
}

CodePudding user response：

I think the only real problem was i, which was unnecessary, as you could also use j for that. Other than that it's pretty clear I think. (I didn't test this code, but it should work.)

int center = width / 2 - 1; //added -1 to make it a bit cleaner later on. Not a big deal though.
for (int i = 0; i <= center; i  )
{
    printf(" ");
}
printf("X\n");

for (int j = center, l = 1; j > 0; j--, l  = 2) // i only existed to make the loop stop, which could easily be shifted to j
{
    for (int h = 0; h < j; h  )
    {
        printf(" ");
    }
    printf("X");

    for(int k = 0; k < l; k  )
    {
        printf(" ");
    }
    printf("X\n");
}