Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
#include <stdio.h>

#define MAXLINE 1000    /* Maximum length of a line */
#define TABSTOP 4       /* Length of tabstop */

int getLine(char line[], int limit);
int lookAhead(char line[], int start, int end);

    /*
     * 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.
     */

int main(void)
{
    int tabIndex, i;
    char line[MAXLINE];

    while ((getLine(line, MAXLINE)) > 0) {
        i = 0;
        while (line[i] != '\0') {
            if (line[i] == ' ') {
                tabIndex = i + (TABSTOP - (i % TABSTOP)); 
                if ((lookAhead(line, i + 1, tabIndex)) == 1) {
                    putchar('\t');
                    i = tabIndex;
                } else {
                    putchar(line[i]);
                    ++i; 
                }
            } else {
                putchar(line[i]);
                ++i;
            }
        }
    }
    return 0;
}

int getLine(char line[], int limit)
{
    int inputVal, i;

    for (i=0; i < (limit - 1) && (inputVal = getchar()) != EOF &&
            inputVal != '\n'; ++i) {
        line[i] = inputVal;
    }
    if (inputVal == '\n') {
        line[i] = inputVal;
        ++i;
    }
    line[i] = '\0';
    return i;
}

int lookAhead(char line[], int start, int end)
{
    int i, clearPath;

    clearPath = 1;
    for (i = start; i < end; ++i) {
        if (line[i] != ' '){
            clearPath = 0;
        }
    }
    return clearPath;
}
share|improve this question

3 Answers 3

I don't think a proper getline() function should return an int, whether it's based on the attempted read (a boolean) or something else.

I believe you're trying to imitate fgets(), which instead returns a char* (the extracted file line) if the read was successful or NULL if it failed. A proper I/O function should set an error flag upon a failed read attempt, and this is not done by returning an int. Plus, there will be issues if the caller of your function expects a string or a failbit, which should be the case if the caller expects proper feedback.

Unless fgets() really doesn't satisfy your needs here, I'd recommend using that or another library function that handles this properly. Recreating an I/O function can cause problems if not written carefully, and even then you shouldn't be doing that.

share|improve this answer
    
The chapter has you build bare bones versions of several std lib functions which is why I included them. I believe the usage of std lib functions will be detailed in later chapters. Perhaps I should have mentioned that. –  ao2130 May 25 at 20:15
    
@ao2130: Yes, that would be a good idea, but at least you've learned something about it. :-) –  Jamal May 25 at 20:35
    
@Jama Curious, do you consider fgets() an improper I/O function because it does not, per C spec, set an error flag anymore than OP's getLine(). Maybe NULL is an error flag in that context? –  chux Jun 7 at 6:15
    
@chux: I think it's okay in regards to error flags. The OP's function, however, doesn't return NULL at all, so the calling code won't know if an error occurred. –  Jamal Jun 7 at 13:44

I believe you have two ways of interpreting "to achieve the same spacing":

  1. Replace consecutive space anywhere in a line.
  2. Replace only spaces at the beginning of a line that occur before any non-space character.

Either way, this is a very difficult problem to solve correctly. You'll need to implement a complete tab-stop interpreter. In addition, with Interpretation #1, you may need to implement a source code parser, if your goal is to be able to retab source code without corrupting literal strings that may be embedded in the text.


Consider the following input text as an example:

/*
A   B   C   D   E   F   G   H   I   J   K   L   M   N   O   P   Q   R
012301230123012301230123012301230123012301230123012301230123012301230123
*/
var␣d␣=␣b␣*␣b␣-␣4␣*␣a␣*␣c;␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣//␣discriminant⏎
if (d␣>=␣0)␣{␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣//␣only␣real␣solutions⏎
␣␣␣␣var␣root1␣=␣(-b␣+␣sqrt(d))␣/␣(2␣*␣a);␣␣␣//␣the␣larger␣root⏎
␣␉  var␣root2␣=␣(-b␣+␣sqrt(d))␣/␣(2␣*␣a);␣␣␣//␣the␣smaller␣root⏎
}⏎

I believe that the first line should get five Tabs, with the first Tab representing just two spaces (G2 and G3). Also, the var root2 line has a superfluous leading space that should be discarded.

With Interpretation #1, the output should be:

var␣d␣=␣b␣*␣b␣-␣4␣*␣a␣*␣c;␉␉␉␉␉//␣discriminant⏎
if (d␣>=␣0)␣{␉␉␉␉␉␉␉␉//␣only␣real␣solutions⏎
␉var␣root1␣=␣(-b␣+␣sqrt(d))␣/␣(2␣*␣a);␉//␣the␣larger␣root⏎
␉var␣root2␣=␣(-b␣+␣sqrt(d))␣/␣(2␣*␣a);␉//␣the␣smaller␣root⏎
}⏎

With Interpretation #2, the output would be:

var␣d␣=␣b␣*␣b␣-␣4␣*␣a␣*␣c;␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣//␣discriminant⏎
if (d␣>=␣0)␣{␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣//␣only␣real␣solutions⏎
␉var␣root1␣=␣(-b␣+␣sqrt(d))␣/␣(2␣*␣a);␣␣␣//␣the␣larger␣root⏎
␉var␣root2␣=␣(-b␣+␣sqrt(d))␣/␣(2␣*␣a);␣␣␣//␣the␣smaller␣root⏎
}⏎

There is a case to be made for interpretation #2, which is more conservative about which spaces are safe to replace while achieving the "same" spacing. Here is a real-life example of a bug that was introduced due to a careless spaces-to-tabs transformation that meddled with a literal string within some source code. If there is any doubt about correctness and safety, I think a good policy is to leave the text untransformed.

share|improve this answer

Minor: int getLine(char line[], int limit)

Following code has trouble if limit is small as inputVal is tested without prior setting and line[i] is not known to be in range.

int inputVal;
...
if (inputVal == '\n') {
    line[i] = inputVal;
    ++i;
}
line[i] = '\0';

Suggest:

// top of function
if (limit < 1) return 0;
int inputVal = 0;
...
if (i < (limit - 1) && inputVal == '\n') {
    line[i] = inputVal;
    ++i;
}

A re-write of this function could be

size_t getLine(char line[], size_t limit) {
    if (limit < 1) return 0;
    limit--;  // Room for \0

    size_t i = 0;
    int inputVal;
    while (i < limit && (inputVal = getchar()) != EOF) {
        line[i] = inputVal;
        ++i;
        if (inputVal == '\n') break;
    }

    line[i] = '\0';
    return i;
}
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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