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

K&R C section 1-20 - removing tabs

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

Problem

1-20 of K&R Exercise 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.

I think my solution is okay. I used the name getlinea when it also "completes" a line when returning from function getlinea because it found a \t.

When I look at the code with


$ cat main.c

And I select the tab spaces of my code that terminal prints, I see indentation spaces as a whole thing, but when I go


$ ./1-20_detab < main.c

I can see the tabs spaces as for regular (not tab) spaces.

```
/*
* main.c
*
* Created on: 22/3/2015
* Author: utnso
*
*Exercise 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?
*
* Answer: n should be a symbolic parameter
*
* Note: 1 )It doesn't say anything about input being lines, i assume they're for simplicity, when testing the program
* 2 ) this program also deletes white lines
*/

#include

#define W_AS_TABS 4

#define MAXLINE 1000

int c;
int len ;

int getlinea ( char line [] );
void replace_tabs ( char tab [] );

int main ( void )
{
extern int len ;
len = 0 ;
extern int c;
char line [MAXLINE] ;
while ( ( len = getlinea ( line ) ) >= 0 ) //getlinea gives a new line or completes a line interrupted when getlinea exited because it found a tab.
if ( c == '\t')
replace_tabs ( line ) ;
else
if ( len != 0 )
{ //not an empty line
printf ("%s\n", line);//if it's the end of a line, show it! ...
len = 0 ; // gimme a new line
}
return 0;
}

int getlinea ( char line [] )
{
extern int c;
extern int len;
while ( ( c = getchar () ) != EOF && c != '\n' && c!= '\t' && len < MAXLINE-1 )
{

Solution

Not everybody uses a tab size of 4

#define W_AS_TABS 4


Might by worth setting this as a default. But the user should be able to change the default.

Global variables are not a good idea:

int c;
int len ;


Prefer to pass as parameters to your function.

Not all lines end with a new line character.

if ( c == EOF )
      return -1;
  return len;


The last line in a file may not work correctly if it is not terminated by a \n.

This is not a correct way to replace tabs.

void replace_tabs ( char s [] )
{
  extern int len;
  int i ;
  for ( i = 0; i < 4  && len < MAXLINE-2 ; ++i ){ //save a position for \0
          s [len] = 32;
          ++ len;
  }
}


You always replace a tab with four spaces. You should replace a tab with the number of spaces to get you to the next tab stop. So a tab character at position 22 and a tab stop of 4 should add only two spaces to get the insert point to position 24.

=======================================================

Added answers to the questions in the comments.


while ( ( c = getchar () ) != EOF && c != '\n' && c!= '\t' && len < MAXLINE-1 )... All lines end up with \n or 998 characters long or EOF...

Yes that bit works.


it has no problem when the final line doesn't end with \n (I tested that)

Are you sure?

if ( c == EOF )
    return -1;


When the last line does not have a '\n' character the return value will be -1. Which means it will not be printed because the main() function has.

while ( ( len = getlinea ( line ) ) >= 0 )


Thus it never enters the part where it does the printing.

Here is my test (and it fails)

# This works as expected because there
# is a new line on the end.
> echo bob is waiting > test1
> cat test1 | ./a.out
bob is waiting

# This fails.
# We create a file were the last line has no '\n'
# The result is no output.
> echo -n "bob is waiting" > test
> cat test | ./a.out



2... len is a global variable so replace_tabs ( line ) can modify this value, and getlinea "knows" it doesn't need to start "getting a new line everytime it's invoked"

I can see how it works.

Does not change the fact that it is bad practice. Global mutable state makes functions harder to reason about (as other functions can mutate the state when you are not looking). Passing a value as a parameter allows you to control the state better.


Could you please explain last paragraph? I didn't quite get it.

Tab characters are not 4 characters long.

They are the length of required to get to the tab stop. So if you have a tab size of 4. Then you have tab stops at 4, 8, 12, 16, 20, 24, 28 ....

So when you replace a tab you don't replace it with four spaces. You replace it with the number of spaces required to get the next character to tab stop.

So if you find a tab at position 22 (ie there are 21 characters in front of it). Then you should replace the tab with only 2 spaces to make the next inserted character appear at position 24 (the next tab stop).

=======================================================


If i replace the tab with two whites characters then It's OK? doesn't make a lot of sense to me.

> echo "XLine1   // Line beginning with tab" | tr 'X' '\t' > test
> echo "  XLine2   // Line beginning with 2 spaces then a tab" | tr 'X' '\t' >> test


If you edit this file in an editor, then the first L on each line will line up one above the other; try it.

> cat test
        Line1   // Line beginning with tab
        Line2   // Line beginning with 2 spaces then a tab


If you run this file through your program. Then the Ls no longer line up. You have broken the indentation of the file. The whole point of the exercise is to make sure you maintain indentation.

> cat test | ./a.out
    Line1   // Line beginning with tab
      Line2   // Line beginning with 2 spaces then a tab


=======================================================

If you don't do the indentation correctly (as I describe above). You can simplify your current program too:

int main()
 {
      while( ( c = getchar () ) != EOF) {
          if (c != '\t') {
               printf("%c", c);
          }
          else {
               printf("    ");
          }
      }
 }

Code Snippets

#define W_AS_TABS 4
int c;
int len ;
if ( c == EOF )
      return -1;
  return len;
void replace_tabs ( char s [] )
{
  extern int len;
  int i ;
  for ( i = 0; i < 4  && len < MAXLINE-2 ; ++i ){ //save a position for \0
          s [len] = 32;
          ++ len;
  }
}
if ( c == EOF )
    return -1;

Context

StackExchange Code Review Q#84736, answer score: 9

Revisions (0)

No revisions yet.