Program to replace tabs with blanks

Question

From my C university book 'The C Programming Language' by Brain W.Kernighan and Dennis M.Ritchie. Excercise 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. Assume a fixed set of tab stops, say every n columns. Should n be a variable or a symbolic parameter?

#include <stdio.h>

#define TABSTOP 8
#define MAXLINE 1000

int getline(char s[], int len);

int main()
{
    char line[MAXLINE];

    while ((getline(line, MAXLINE) > 0))
    {
        int i, j, count;

        count = 0;
        for (i = 0; line[i] != '\0'; ++i)
        {
            if (line[i] == '\t')
            {
                for (j = 0; j < TABSTOP - count; ++j)
                {
                    putchar(' ');
                }

                count = 0;
            }
            else
            {
                ++count;

                if (count >= TABSTOP)
                    count = 0;

                putchar(line[i]);
            }
        }
    }

    system("pause");

    return 0;
}

int getline(char line[], int len)
{
    int c, i;

    for (i = 0; i < len - 1 && (c = getchar()) != EOF && c != '\n'; ++i)
        line[i] = c;

    if (c == '\n')
    {
        line[i] = c;
        ++i;
    }

    line[i] = '\0';

    return i;
}

My concerns are that I didn't get the question right, did they mean that every time I see a tab in the input I need to put the corresponding amount of spaces instead of tabs?

Are there any flaws or bugs in the code?

I didn't understood what is a 'symbolic parameter', did they mean symbolic constant?

By 'n' did they mean what I did with the 'TABSTOP' symbolic constant?

Answer 1

Headers and Names

• You are using the system function that is declared in stdlib.h but you don't #include <stdlib.h>.
• The name getline is already taken by many C implementations for an extension to the standard library's I/O functions. You shouldn't use it for your own function names.

Program structure

Your program basically consists of the main function. While simple it is, this is not a very modular design and in particular hostile to unit testing. I suggest you decouple the I/O logic from the tab expansion logic. Have one function read the input line-by-line and for each line it reads, make it call another function that processes the line and places the results in a separate buffer. It could look like this:

size_t
expand_tabs(int tabwidth, const char * input, char * output, size_t length);

The first function then outputs that buffer. This way you can, for example, unit test expand_tabs with an array of strings.

Don't put

system("pause");

in your programs. I know many tutorials targeted at people using the Windows operating system suggest this but it is poor advice. If your program is done, make it exit. If this causes your terminal emulator to close the window before you wanted it to, fix the way you use the terminal emulator, not your program.

I/O Handling

Avoid character-based I/O. It's slow and painful. Instead of your hand-rolled getline function, you could have used the standard library fgets function. From its man page:

Synopsis

#include <stdio.h>

char *fgets(char *s, int size, FILE *stream);

Description

fgets() reads in at most one less than size characters from stream and stores them into the buffer pointed to by s. Reading stops after an EOF or a newline. If a newline is read, it is stored into the buffer. A terminating null byte ('\0') is stored after the last character in the buffer.

Return Value

fgets() returns s on success, and NULL on error or when end of file occurs while no characters have been read.

This is just what your getline function does but faster and cleaner.

Never use the seemingly simpler gets function. It is broken and C11 removed it from the standard library.

While using stack-allocated buffers like

char line[MAXLINE];

is fast, the hard-coded limit is a little awkward. You actually took care to make your program process files with lines longer than MAXLINE correctly too. Congratulations for that. Another option would be to use a dynamically allocated buffer. If your implementation has it, the aforementioned getline function comes in handy, here. If you are following the book, I'm not sure if you've already learned about dynamic memory yet, though.

Style

While declaring functions as

int my_getline(char s[], int len);

is valid syntax, it is misleading because the first argument actually is a pointer and not an array. So I prefer the (functionally equivalent)

int my_getline(char * s, int len);

syntax.

As long as you want to use MAXLINE to create an array, it needs to be a macro, unfortunately. For TABSTOP there is no such need. Avoid macros if possible. Here, a static const int would have been perfectly fine. Note that in the expand_tabs function suggested above, I'm even passing it as a parameter so it doesn't depend on global constants. Of course, the constant has to be coded at some point in your program.

Answer 2

includes and symbol-names

5gon12eder is right about the missing header for system and many implementations claiming the symbol getline.
Just want to add that if the implementation defining getline interferes with you defining it however you want, it's non-conforming, for whatever that's worth.

Forward-declarations

Don't repeat yourself.
If there are no circular dependencies, omit any forward-declarations and instead define before use.

The above should not be as a vote against using multiple source-files and declarations in included header-files where the projects size merits that.
Though even (or especially) in that case, just like there is only one definition, there should only be a single declaration.

system("pause");

As 5gon12eder said, prune this. If your terminal quits before you see the output, fix how you call your program.

getline

Let's for the moment pretend reading each separate line into a buffer and then transforming that was a good idea, and there was no standard-library function for that.

Well, you should avoid pushing so much into a for-loop's condition, even though it technically works.
Also, slightly changing the loop allows merging the deferred storing of the newline.
I also added a debug-mode precondition-check (#include <assert.h>):

int getline(char line[], int len) {
    assert(line && len > 0);
    int c = 0, i = 0;
    while(c != '\n' && i < len - 1 && (c = getchar()) != EOF)
        line[i++] = c;
    line[i] = 0;
    return i;
}

How big is a tabstop?

People use tabstops of different lengths, the classic 8 being most common, 4 following quite a long step behind, 2 also being seen, and that was nearly everything observed.
So allow the user to override the default.

I/O and Algorithm

Here we get to the heart of it.

One could go byte-at-a-time (just drop getline and use getchar / putchar directly), even though that's incredibly inefficient.
There are also disadvantages to going line-at-a-time (using fgets, gets is always an error): Lines can be too long for your buffer (and simply growing the buffer is no good), or too short for efficiency (degenerates to byte-at-a-time) and you don't need the whole line anyway.
The optimal unit of work is block-at-a-time (using fread, fwrite, and reasonably-sized buffers (powers of two)). Beside being more efficient (single-pass, aside from newline-transformation if neccessary on the implementation), it also handles embedded 0-bytes gracefully.

#include <stdio.h>
#include <stdlib.h>
#define BUFFSIZE (1<<11)
#define MAXTABSTOP 64

int main(int c, char* argv[]) {
    char input[BUFFSIZE];
    char output[BUFFSIZE+MAXTABSTOP];
    int n = 0, i = -1, o = 0, t = 0, tabstop = 8;
    if(c > 1) {
        tabstop = strtol(argv[1], argv, 10);
        if(**argv || tabstop < 1 || tabstop > MAXTABSTOP)
            tabstop = 8;
    }
    for(;;) {
        if(n == ++i && (i = 0) == (n = fread(input, 1, BUFFSIZE, stdin))) {
            fwrite(output, 1, o, stdout);
            return 0;
        }
        switch(input[i]) {
        case '\t':
            if(!t)
                t = tabstop;
            while(t--)
                output[o++] = ' ';
            break;
        case '\n':
            t = -1;
            // FALLTHRU
        default:
            t = (t+1) % tabstop;
            output[o++] = input[i];
        }
        if(o >= BUFFSIZE) {
            fwrite(output, 1, o, stdout);
            o = 0;
        }
    }
}