patterncMinor
Program to replace tabs with blanks
Viewed 0 times
blankswithreplaceprogramtabs
Problem
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?
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?
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
#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 = 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?
Solution
Headers and Names
Program structure
Your program basically consists of the
The first function then outputs that buffer. This way you can, for example, unit test
Don't put
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
Synopsis
Description
Return Value
This is just what your
Never use the seemingly simpler
While using stack-allocated buffers like
is fast, the hard-coded limit is a little awkward. You actually took care to make your program process files with lines longer than
Style
While declaring functions as
is valid syntax, it is misleading because the first argument actually is a pointer and not an array. So I prefer the (functionally equivalent)
syntax.
As long as you want to use
- You are using the
systemfunction that is declared instdlib.hbut you don't#include.
- The name
getlineis 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
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.Code Snippets
size_t
expand_tabs(int tabwidth, const char * input, char * output, size_t length);system("pause");#include <stdio.h>
char *fgets(char *s, int size, FILE *stream);char line[MAXLINE];int my_getline(char s[], int len);Context
StackExchange Code Review Q#106883, answer score: 5
Revisions (0)
No revisions yet.