HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

K&R 1-20 Solution

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
solutionstackoverflowprogramming

Problem

I'm currently reading the infamous K&R C book and trying to solve Exercise 1-20. My solution looks kind of too simple, but it works. I've searched the web for different solutions, but they all are much longer, although I didn't see any improvement compared to my code. Do you see anything which might compromise my approach? The exercise is as follows:


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?

And here's my solution:

#include 
#define COLS 8

int main (void)
{
    int ch;
    int charCounter = 0;

    while ((ch = getchar()) != EOF) {
        if (ch != '\t'){
            putchar(ch);
            charCounter++;
        }

        if (ch == '\t'){
            for (int i = 0; i < (COLS - (charCounter % COLS)); ++i) {
                putchar(' ');
            }
            charCounter = 0;
        }

        if (ch == '\n'){
            charCounter = 0;
        }
    }
}

Solution

Do you see anything which might compromise my approach?

-
Good use of int ch; as the type returned by fgetc() is int and not char - avoided a rookie mistake.

-
The if() layout looks weak. Suggest using else

// current
if (condition) {
}
if (!condition) {
}

// Suggest
if (condition) {
}
else {
}


-
It is easier for people to understand == rather than !=. This is important for writing the correct code and maintaining it. So, when practical, avoid negation. Don't you think it is not a bit mis-understandable to negate the inverse of a sentence, No?

if (ch == '\t') {
  for (int i = 0; i < (COLS - (charCounter % COLS)); ++i) {
    putchar(' ');
  }
  charCounter = 0;
} else {
  putchar(ch);
  charCounter++;
}


-
The if (ch == '\n') is only possible when ch != '\t'. But this is minor. In a more complex code having this if() stand alone may be preferable.

...
} else {
  putchar(ch);
  charCounter++;
  if (ch == '\n') {
    charCounter = 0;
  }
}


-
Then there is the missing return _something_;. With main(), lack of a return in the end will inject a return 0;. Should you do this or not is a coding style. One side says, no. Minimal code. The other side says - yes. Be explicit, it is poor practice to omit. The larger point here is that some "do/don't do" axioms are driven by your group's coding style. Following a consistent style is more important than being "right". If your group lacks a coding style, create one. This applies to all sorts of indentation, brackets or not, ++i or i++ issues.

-
Pedantic code would avoid integer overflow from pathological long lines. Example:

// charCounter++;
 charCounter = (charCounter + 1)%COLS;

 // i < (COLS - (charCounter % COLS)_;
 i < (COLS - charCounter);


-
Clerical: Since the design document is "K&R C, Exercise 1-20.", that information should have been in code. Name and date is nice. It is your work, proudly sign it,

/* K&R C,  Exercise 1-20. */
/* Saalim  2016 Apr 5 */

#include 
...



Should n be a variable or a symbolic parameter?

A more advance code would code some means to pass in a variable rather than a fixed tab stop of 8.

Very good initial post.

Code Snippets

// current
if (condition) {
}
if (!condition) {
}

// Suggest
if (condition) {
}
else {
}
if (ch == '\t') {
  for (int i = 0; i < (COLS - (charCounter % COLS)); ++i) {
    putchar(' ');
  }
  charCounter = 0;
} else {
  putchar(ch);
  charCounter++;
}
...
} else {
  putchar(ch);
  charCounter++;
  if (ch == '\n') {
    charCounter = 0;
  }
}
// charCounter++;
 charCounter = (charCounter + 1)%COLS;

 // i < (COLS - (charCounter % COLS)_;
 i < (COLS - charCounter);
/* K&R C,  Exercise 1-20. */
/* Saalim  2016 Apr 5 */

#include <stdio.h>
...

Context

StackExchange Code Review Q#124866, answer score: 4

Revisions (0)

No revisions yet.