My first impression is that the code is very hard to read. There are lots of variables all with single-letter identifiers, without even comments to help understand what each one represents. As a general rule, a variable's descriptiveness should be proportional to the size of its scope. --- `void main()` is not a portable declaration of `main()` - if your compiler doesn't at least warn about this, you probably haven't enabled enough diagnostics. Portable definitions of `main()` all return `int` (though you don't explicitly need to `return` a value - reaching the end of `main()` will cause the runtime to exit with a success status value). --- Variable `int a` is declared but never used. --- If `printf("ENTER OUTER NUMBER:")` fails, it seems foolish to try reading input. Don't ignore function return values unless you've considered the consequences! Even more significantly, if the `scanf()` call fails to convert its input to `int`, then we should not be continuing with uninitialised value of `n`. Consider accepting the size as a command-line argument, rather than requiring it to be passed via standard input. You could use the existing logic as a fallback if the argument isn't provided. Having said that, we should think about what range of `n` we're willing to accept. Consider this invocation: ``` ./207421 <<<12 ENTER OUTER NUMBER:1212121212121212121212121212121212121212121212 1211111111111111111111111111111111111111111112 1211101010101010101010101010101010101010101112 12111099999999999999999101112 12111098888888888888889101112 12111098777777777777789101112 12111098766666666666789101112 12111098765555555556789101112 12111098765444444456789101112 12111098765433333456789101112 12111098765432223456789101112 12111098765432123456789101112 12111098765432223456789101112 12111098765433333456789101112 12111098765444444456789101112 12111098765555555556789101112 12111098766666666666789101112 12111098777777777777789101112 12111098888888888888889101112 12111099999999999999999101112 1211101010101010101010101010101010101010101112 1211111111111111111111111111111111111111111112 1212121212121212121212121212121212121212121212 ``` If we're to assume that each number has width of 1 character, then we need to ensure 0 < `n` < 10. Alternatively, we could specify a field width when we `printf()` each value, so that all values are printed consistently wide. We could obtain the required width for the largest value using `snprintf(NULL, 0, …)`. Even then, we might to limit the width for usability reasons - particularly if standard output is connected to a terminal! --- Splitting the output up over several loops makes it very hard to follow. I'd prefer to use a single pair of nested loops to write all the output: ``` for (unsigned y = 0; y < width; ++y) { for (unsigned x = 0; x < width; ++x) { printf("%u ", distance_from_centre(size, x, y)); } puts(""); } ``` All we have to do is write a suitable `distance_from_centre` function that returns the ring number for a given position. --- # Improved code Applying all the suggestions above, we get: ``` #include <stdio.h> #include <stdlib.h> #include <string.h> /* Read a line from standard input, removing the final newline. Returns number of characters read, zero on failure. */ static size_t read_string(char *buf, size_t buf_size, char const *prompt) { if (printf("%s: ", prompt) < 2) { return 0; } fflush(stdout); if (!fgets(buf, sizeof buf_size, stdin)) { return 0; } size_t len = strlen(buf); if (len = 0 || buf[len-1] != '\n') { return 0; } buf[len-1] = '\0'; return len; } /* Find which ring is at the position (x, y) for a set of concentric rings where 1 is the inner point and rings is the outer ring. */ static unsigned distance_from_centre(unsigned rings, unsigned x, unsigned y) { unsigned dx = x >= rings ? x - rings + 2 : rings - x; unsigned dy = y >= rings ? y - rings + 2 : rings - y; return dx > dy ? dx : dy; } /* Convert arg to number, and print that many concentric rings */ static unsigned print_rings(char const *arg) { /* Convert input; allow only 1-99 rings */ char *end; unsigned long const rings_l = strtoul(arg, &end, 10); if (*end || rings_l - 1 >= 99) { fprintf(stderr, "Invalid ring count %s\n\n", arg); return 0; } unsigned const rings = (unsigned)rings_l; /* Do the printing */ int const field_width = snprintf(NULL, 0, "%u", rings); unsigned const width = 2 * rings - 1; for (unsigned y = 0; y < width; ++y) { for (unsigned x = 0; x < width; ++x) { printf("%*u ", field_width, distance_from_centre(rings, x, y)); } puts(""); } puts(""); return rings; } int main(int argc, char **argv) { if (argc < 2) { /* No arguments: fall back to reading from input */ char buf[10]; /* plenty for valid sizes, including newline and null */ if (!read_string(buf, sizeof buf, "Enter number of rings")) { fprintf(stderr, "Failed to read input\n"); return EXIT_FAILURE; } print_rings(buf); } else { while (*++argv) { print_rings(*argv); } } } ``` You'll notice that much of this code is for robust input and other error-checking; the actual printing code is relatively small. That tends often to be the case when writing C programs.