Printing a text rectangle

Question

I'm starting C as my first language and am just under halfway done the book I'm using. However, I commonly hear about 'bad' programmers and 'bad' code whenever I encounter any sort of programming related discussion on the internet.

I've done some examples that do work as they should but, I want to make sure that when I eventually am able to contribute to projects on GitHub, etc. that I'm not screwing everything up and writing unmaintainable code.

Do these examples follow common best practices? I did have comments but took them out as the code should be easy enough to understand and it's the actual code I'm wondering about.

/*make a box*/
#include <stdio.h>
int main(void)
{   
    int i = 0;  
    int long length = 0;
    int long width = 0; 
    printf("Enter Box dimensions (LxW):");
    scanf("%ld %ld", &length, &width);
    for(i = 0; i <= width; i++)
    {
        printf("*");
    }
    for(i = 0; i <= length; i++)
    {
        printf("\n*");
        for(int a = 0; a <= (width-2); a++)
        {
            printf(" ");
        }
        printf("*");
    }
    printf("\n");
    for(i = 0; i <= width; i++)
    {
        printf("*");
    }
    return 0;
}

Answer 1

Dimensions and looping

It looks like you have some off-by-one errors. This is not a 1×7 box by any definition that I can think of:

Enter Box dimensions (LxW):1 7
********
*      *
*      *
********

In C, the idiomatic way to write a loop that does something n times is for (int i = 0; i < n; i++) { … }. (Sometimes, the situation calls for for (int i = 1; i <= n; i++) { … } instead, but that's rarer.) You start your counters at 0, which is good, but you used <= in your termination conditions, which is weird.

Functions

It's a good habit to make a function for any chunk of code that has an identifiable purpose. Here, it makes sense to define a void printBox(int length, int width) function. If you need to modify the program to take command-line parameters instead of keyboard input, then you can still call the same function.

By the way, int long is a bit unconventional. Unless you have a specific reason to do otherwise, just use int.

Smarter library use

You're basically using printf() where the simpler putchar() function will do. The vertical lines, though, can be better written using a smarter call to printf(). (Consult the documentation for printf().)

Error checking

Functions like scanf() can fail due to invalid user input. You should be in the habit of checking the return value.

Suggested solution

#include <stdio.h>

void printBox(int length, int width) {
    // Top
    if (length >= 1) {
        for (int i = 0; i < width; i++) {
            putchar('*');
        }
        putchar('\n');
    }

    // Sides
    for (int i = 0; i < length - 2; i++)
    {
        printf("*%*s\n", width - 1, "*");
    }

    // Bottom
    if (length >= 2) {
        for (int i = 0; i < width; i++) {
            putchar('*');
        }
    }
    putchar('\n');
}

int main(void)
{   
    int length, width;
    printf("Enter Box dimensions (LxW):");
    if (2 != scanf("%d %d", &length, &width)) {
        // Indicate input error
        return 1;
    }
    printBox(length, width);
}

Answer 2

I was going to write more but @200_success already said most of what I was going to say. So I'll just add this:

Choice of types

Don't use int long or long int: just use long, which means the same thing. Also, I'm not sure why you chose to use long over int here, since I don't think you will want to print a box with a dimension larger then 32K. Since your dimensions should not be negative, I would either use unsigned long or check that the inputs were not negative.

Answer 3

Building on top of @200_success good answer, he still has a bit a code duplication. I suggest the further extraction of:

 void print_line(int length) {
      for (int i = 0; i < length; i++) {
          putchar('*');
      }
 }