0

I bought a C book called "The C (ANSI C) PROGRAMMING LANGUAGE" to try and teach myself, well C. Anyhow, the book includes a lot of examples and practices to follow across the chapters, which is nice.

Anyhow, the code below is my answer to the books "count the longest line type of program", the authors are using a for-loop in the function getLine(char s[], int lim). Which allows for a proper display of the string line inside the main() function. However using while won't work - for a reason that is for me unknown, perhaps someone might shed a light on the situation to what my error is.

EDIT: To summarize the above. printf("%s\n", line); won't display anything.

Thankful for any help.

#include <stdio.h>
#define MAXLINE 1024

getLine(char s[], int lim) {
    int c, i = 0;

    while((c = getchar()) != EOF && c != '\n' && i < lim) {
        s[++i] = c;
    }

    if(c == '\n' && i != 0) {
        s[++i] = c;
        s[++i] = '\0';
    }    
    return i;
}  

main(void) {
    int max = 0, len;
    char line[MAXLINE], longest[MAXLINE];

    while((len = getLine(line,MAXLINE)) > 0) {
        if(len > max) {
            max = len;
            printf("%s\n", line);
        }
    }
    return 0;
}
Improve this question
4
  • 2
    Please explain in which way it doesn't work - fails to compile, fails to run at all, produces incorrect result or fails with an error code, and please include sample output and error output in your question. Commented Jan 26, 2010 at 17:24
  • Not really looked at, but I'd guess it's a \n vs \p problem? Commented Jan 26, 2010 at 17:27
  • you also need to terminate your string with a \0 in the case where you have hit EOF without a \n, otherwise your program will malfunction or crash if your file does not end in a \n Commented Jan 26, 2010 at 17:29
  • 1
    Your code does not work because there is a problem. If you want a more specific answer, ask a more specific question. Commented Jan 26, 2010 at 17:30

5 Answers 5

Reset to default
1

You have a number of serious bugs. Here's the ones I found and how to fix them.

change your code to postincrement i to avoid leaving the first array member uninitialised, and to avoid double printing the final character:

s[++i] = c;
...
s[++i] = c;
s[++i] = '\0';

to

s[i++] = c;
...
// s[++i] = c; see below
...
s[i++] = '\0';

and fix your EOF bug:

if(c == '\n' && i != 0) {
    s[++i] = c;
    s[++i] = '\0';
}

to 

if(c == '\n')
{
    s[i++] = '\n';
}
s[i] = '\0'

Theory

When writing programs that deal with strings, arrays or other vector-type structures it is vitally important that you check the logic of your program. You should do this by hand, and run a few sample cases through it, providing sample inputs to your program and thinking out what happens.

The cases you need to run through it are:

  • a couple general cases
  • all the edge cases

In this case, your edge cases are:

  • first character ever is EOF
  • first character is 'x', second character ever is EOF
  • first character is '\n', second character is EOF

  • first character is 'x', second character is '\n', third character is EOF

  • a line has equal to lim characters

  • a line has one less than lim characters
  • a line has one more than lim characters

Sample edge case

first character is 'x', second character is '\n', third character is EOF

getLine(line[MAXLINE],MAXLINE])
(s := line[MAXLINE] = '!!!!!!!!!!!!!!!!!!!!!!!!...'
c := undef, i := 0
while(...)
c := 'x'
i := 1
s[1] := 'x' => s == '!x!!!!...' <- first bug found
while(...)
c := '\n'
end of while(...)
if (...)
(c== '\n' (T) && i != 0 (T)) = T
i := i + 1 = 2
s[2] = '\n' => s == '!x\n!!!!'
i := i + 1 = 3
s[3] = '\0' => s == '!x\n\0!!!' <- good, it's terminated
return i = 3
(len = i = 3) > 0) = T (the while eval)
if (...)
len (i = 3) > max = F
max = 3 <- does this make sense?  is '!x\n' a line 3 chars long?  perhaps.  why did we keep the '\n' character? this is likely to be another bug.
printf("%s\n", line) <- oh, we are adding ANOTHER \n character?  it was definitely a bug.
outputs "!x\n\n\0" <- oh, I expected it to print "x\n".  we know why it didn't.
while(...)
getLine(...)
(s := line[MAXLINE] = '!x\n\0!!!!!!!!!!!!!!!!!!!...' ; <- oh, that's fun.
c := undef, i := 0
while(...)
c := EOF
while terminates without executing body
(c == '\n' && i != 0) = F
if body not executed
return i = 0
(len = i = 0 > 0) = F
while terminates
program stops.

So you see this simple process, that can be done in your head or (preferably) on paper, can show you in a matter of minutes whether your program will work or not.

By following through the other edge cases and a couple general cases you will discover the other problems in your program.

Improve this answer
Sign up to request clarification or add additional context in comments.

6 Comments

note that this code will count \n in i. you can fix that yourself.
Thanks for the help (and patience), i really appreciate it.
Acknowledging your edit above - it's very helpful and good practice - however this is merely the first chapter, which has not yet touched this subject quite deeply yet.
However - as an advice - even when writing these sorts of practice applications inside a book. Do you recommend to write for the "extreme edge cases"?
I don't know what you mean by extreme edge... isn't an edge extreme by definition? or do you mean very very unlikely edge cases? none of the edge cases I listed are very unlikely.
|
0

It's not clear from your question exactly what problem you're having with getLine (compile error? runtime error?), but there are a couple of bugs in your implementation. Instead of 

s[++i] = something;

You should be using the postfix operator:

s[i++] = something;

The difference is that the first version stores 'something' at the index of (i+1), but the second version will store something at the index of i. In C/C++, arrays are indexed from 0, so you need to make sure it stores the character in s[0] on the first pass through your while loop, in s[1] on the second pass through, and so on. With the code you posted, s[0] is never assigned to, which will cause the printf() to print out unintialised data.

The following implementation of getline works for me:

int getLine(char s[], int lim) {

    int c;
    int i;

    i = 0;

    while((c = getchar()) != EOF && c != '\n' && i < lim) {
        s[i++] = c;
    }

    if(c == '\n' && i != 0) {
        s[i++] = c;
        s[i++] = '\0';
    }

    return i;
}
Improve this answer

1 Comment

your version will still bomb. why? because the last input line is not null terminated. proof: c == EOF && i = 0 => !(c=='\n' && i!=0) => s[i-1] != \0. c == EOF && i != 0 => c != '\n' => !(c=='\n' && i!=0) => s[i-1] != \0. bang.
0

By doing ++i instead of i++, you are not assigning anything to s[0] in getLine()!

Also, you are unnecesarilly incrementing when assigning '\0' at the end of the loop, which BTW you should always assign, so take it out from the conditional.

Improve this answer

Comments

0

Also add return types to the functions (int main and int getLine)

Improve this answer

Comments

0

Watch out for the overflow as well - you are assigning to s[i] at the end with a limit of i == lim thus you may be assigning to s[MAXLINE]. This would be a - wait for it - stack overflow, yup.

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.