Kernighan & Ritchie - detabbing program

Question

Task (1.20)

Write a program detab that replaces tabs in the input with the proper number of blanks to space to the next tab stop.

Also, the condition is is not to use pointers and structs etc because they will be considered in next chapters.

I'm new in C, and want to know what in my solution is bad idea and what can be done other way (but without pointers):

#include <stdio.h>
#define TABLINE 4

int count(char stri[]);

void main() {

    printf ("\nTabline is %d\n", TABLINE);

    int i = 0;
    int h;
    char stri[100];
    char c;

    while ((c = getchar()) != '\n' && c != EOF) {
        if (c == '\t')
            for (h=0; h<TABLINE; ++h) {
                stri[i] = ' ';
                ++i;
        } else {
            stri[i] = c;
            ++i;
        }
    }

    printf ("%d\n", count(stri));

    for (i=0; i< count(stri); ++i)
        printf ("%c", stri[i]);
}

int count(char stri[]) {

    int l;

    for (l=0; stri[l]; ++l)
        ;

    return l;
}

And its work:

$ ./detab

Tabline is 4
befortan        aftertab
20
befortan    aftertab

Answer 1

Apart from the fact that a tab doesn't necessarily map to 4 spaces, as was nicely put by @Loki in his comment, there are a few other points that you can improve:

• Your main function is not following a standard signature. It should be returning an int.

• Your count() function, besides having a vague name (what is it counting exactly?), is reinventing strlen(). Avoid reinventing the wheel, unless you have a good reason to (in this case I'd say yes, since you are learning, but it is also valuable to know the standard functions and how to use them).

• Still talking about count(), avoid calling it in the conditional section of a loop, like here:

   for (i=0; i< count(stri); ++i)
  

  Remember that the condition gets tested every iteration of the loop, so this would imply a full scan of stri if the compiler is not smart enough to hoist that function call out the loop. Better would be to call the function once and store its result in a local variable.

• Declare variables as close to their first usage as it is possible. For instance, h could be declared inside the body of the for loop:

  for (int h = 0; h < TABLINE; ++h)
  

  Reducing scope is important to avoid name collisions and facilitate the understanding of the program states. If you had to remember the state of all variables in a program, you would be doomed. Variables that are scoped inside a { } block can be forgotten as soon at the block ends. There is a discussion here that might also find relevant.

• You make two other dangerous assumptions in your program: One is that the input will never exceed one line (you terminate the loop in the first '\n') and that the input will never exceed 100 characters (that's the size of your string buffer). Those assumptions are very unrealistic. More than likely the input would consist of multiple lines in a real world scenario. A 100 chars limit is also quite small. If using a statically allocated buffer, without resorting to malloc and pointers, at least make it a bit bigger and make sure to end the loop when the buffer is full. Something in the lines of (where MAX_CHARS is the size of your char[] buffer):

  while (i < MAX_CHARS && (c = getchar()) != '\n' && c != EOF) {
      ...
  }
  

• Lastly, there is a confusion with the curly braces in the main loop which I'm not quite sure how the compiler is resolving, but it certainly is broken:

  while ((c = getchar()) != '\n' && c != EOF) { 
      if (c == '\t')  
          for (h=0; h<TABLINE; ++h) { 
              stri[i] = ' ';
              ++i;
      } else { // <--- does this belong to the first if ???
          stri[i] = c;
          ++i;
      } 
  } 
  

  As you can see if you look carefully, the } else part is bound to the { in the for loop, the if has no {. I was surprised that this compiled, but it did!

  Be very careful with this. My advice is to always add curly braces on all control statements and loops, even for the single line ones.