5
\$\begingroup\$

I'm new to C, just started reading K&R C book, and am working through exercises. This is my solution to 1-21, and as far as I tested it works. Anything that I'm doing wrong, or that it's not idiomatic C? What can be improved ?

Exercise 1-21. Write a program entab that replaces strings of blanks by the minimum number of tabs and blanks to achieve the same spacing. Use the same tab stops as for detab. When either a tab or a single blank would suffice to reach a tab stop, which should be given preference?

#include <stdio.h>

#define TAB_SIZE 4

int main_entab() {
    int c;
    int character_count = 0;
    int whitespace_count = 0;
    while ((c = getchar()) != EOF) {

        if (c == ' ') {

            // A _ B _ | _ C _ _ | 
            int numberOfCharsToReachTabStop = (TAB_SIZE - ((character_count + whitespace_count) % TAB_SIZE));

            whitespace_count++;

            if (numberOfCharsToReachTabStop == 1 && whitespace_count > 0) {
                putchar('\t');
                //advance count by how many whitespaces we are replacing, so next tab stop calculation is correct
                character_count = character_count + whitespace_count;
                whitespace_count = 0;
            }


        } else {
            //we encountered a non-whitespace char. Print all 'saved' whitespaces.
            while (whitespace_count > 0) {
                putchar(' ');
                whitespace_count--;
                character_count++;
            }

            //print the non-whitespace char
            character_count++;
            putchar(c);
        }
    }
}
\$\endgroup\$
  • \$\begingroup\$ the tab size on a printer or terminal will be 8 characters, not 4 characters, so the posted code will fail to perform as desired \$\endgroup\$ – user3629249 Feb 15 at 0:53
  • \$\begingroup\$ the main() function name is not optional. It must be 'main' not 'main_entab' \$\endgroup\$ – user3629249 Feb 15 at 0:55
7
\$\begingroup\$

Constants

#define TAB_SIZE 4

This is the old way of doing things. Modernly C uses the same syntax as C++:

const int TAB_SIZE = 4;

While both will still function, this has the added benefit of offering type safety. The #define is just a search and replace used by the preprocessor. I.e. it replaces TAB_SIZE with the character 4 in an intermediate copy of the file. The const variable will be managed by the compiler.

Update assignments

                character_count = character_count + whitespace_count;

This is more idiomatically written

                character_count += whitespace_count;

That form is exactly the same as the longer form. It evaluates the left-hand side of the assignment, adds it to the right-hand side, and stores the result in the left-hand side. But it is shorter and more easily recognizable.

Simplifying logic

However, I wouldn't do either of these. Instead, on each iteration of the loop, just increment character_count. Then you never have to add character_count and whitespace_count. This will work in your program because you never use character_count separately from whitespace_count. So make this change and replace character_count + whitespace_count with just character_count.

Yoda conditions

        if (c == ' ') {

This is often written as

        if (' ' == c) {

The reason is that if you accidentally leave off an equals sign ' ' = c will generate a compiler error. Meanwhile, c = ' ' will silently do the wrong thing. It wouldn't be so bad here, as it will be immediately obvious in the display. But in a different kind of program, assigning to c might make a much more subtle bug.

Correct naming

            //we encountered a non-whitespace char. Print all 'saved' whitespaces.

This is incorrect. A whitespace is any character that can produce indent. It specifically includes tabs (and new lines). But what you check is non-space characters. This is easy enough to fix, just remove "white" both times here and rename whitespace_count to space_count.

Consistent naming style

You have the camelCased numberOfCharsToReachTabStop and snake_cased character_count and whitespace_count. Please pick one kind of casing per identifier type and stick to it. If local variables are snake_cased, then number_of_chars_to_reach_tab_stop. Or change them all to camelCase. I personally prefer snake_case, which is easier for non-native English speakers, as it doesn't rely on an ability to differentiate between capital and lowercase letters. But the most important thing is to be consistent so that people can see snake_case and realize that it is a local variable or one of the other things that use snake_case.

TAB_SIZE is a constant, so it is appropriate to use a different casing style (e.g. ALL_CAPS) for it.

Bugs

As already noted, you don't print spaces at the end of a line. But there is a more serious bug. You treat tabs as single characters in the character count. You should not. Instead, tabs are TAB_SIZE characters. Don't forget to adjust for the tab stop. I.e. if you are currently one character past the tab stop, you'd only add three characters for the tab. So if you adopt my previous suggestion of incrementing character_count on every iteration, you need something like

if ('\t' == c) {
    /* update character_count appropriately */
} else {
    character_count++;
}

I'll leave the actual update to you, as that's the exercise that you're exploring.

\$\endgroup\$
  • \$\begingroup\$ Thank you so much for the detailed answer :) \$\endgroup\$ – Rick Sanchez Feb 12 at 23:33
3
\$\begingroup\$

After whitespace_count++, the condition && whitespace_count > 0 will always be true, so can be removed.

Bug: if the file ends in space characters, these can be lost.

\$\endgroup\$
  • \$\begingroup\$ Unless INT32_MAX == whitespace_count. I would NOT advice to remove the condition. \$\endgroup\$ – Nicolas Feb 13 at 13:39
  • 2
    \$\begingroup\$ @Nicolas whitespace_count is always non-negative. It could only overflow if TABSIZE == INT_MAX+1 which is already an overflow. \$\endgroup\$ – AJNeufeld Feb 13 at 14:13
1
\$\begingroup\$

the tab size on a printer or terminal will be 8 characters, not 4 characters, so the posted code will fail to perform as desired

the main() function name is not optional. It must be 'main' not 'main_entab'

\$\endgroup\$
  • \$\begingroup\$ Tab size I think is configurable. It's 4 on my machine. My guess is there should be a way to get the current size. Yes, I know about the main, I was calling the main_entab() from another file that has the main function. \$\endgroup\$ – Rick Sanchez Feb 15 at 9:57

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.