Newbie cant see why code is erroring, but knows where it is erroring

Question

I have a problem with my code, good news is I actually pinpointed the problem, bad news is I do not understand why it is a problem. Also should this be return or exit? This is my getNums() function,...so far. first my code calls getLine() which gets the line and returns its character length. Then get nums is given, the line, length of the line, and an empty array to put numbers in. and is suppose to return the number of numbers it just put in.

int getNums(char s[], int endMarker, int numarray[])
{
    char c;
    double value;
    int counter =0;
    int i,j;
    for(i=0;i<endMarker;i++) {
        while ((c=s[i]!='\n')&&(c!=' ')) {
            //errors
            if ( (c<'0') || (c>'9') ) {
                return(-1); //was exit testing return, **this always kicks me out**
                if( counter > 6){
                    return(-2);
                } //was exit testing return
                s[i]=c;
                i++;
                value = value*10+'c'-'0';
            }else
                numarray[j]=value;
            j++;
            counter++;   
        }
        if ((c=getchar())==' ') {      
            i++;
        }
    }
    return (counter);
    printf("%c,%c:",counter,value); // for testing
}

Answer 1

There are a couple of issues with your code (aside from it not being indented properly). I'll address them as I see them.

I don't think you want a while loop in your for loop. You are already iterating through every character in the string with the for. Perhaps you want to check each character against 'n' and ' ', for which you would simply use an if.

On second glance, it looks like you're trying to iterate through the string twice, because you have two loops and you do i++ in two different places. This leads me to believe that you haven't quite thought out your algorithm. I think that you should sit down and write up some pseudocode. List step-by-step what you want your function to accomplish in plain english and make sure it makes sense logically, before you even write a line of code.

Also, if you return from a function, that function will stop executing. So if you just want to skip over executing an iteration of a loop, use continue instead. Looking at your code I think you might have a return where you really mean to have a continue.

Can you indent your code to make it a bit more readable and easier to follow?

Answer 2

Just glancing at the code, it seems to have a number of problems in a number of places. At least IMO, the first (and probably biggest) problem is that it should really be broken up into a number of separate functions.

Second, the formatting is quite poor. For example the indentation makes it it look like some statements are controlled by an if that really aren't.

Third, you're mismatching types, passing an int and a double to printf, but using the %c (char) conversion for both.

There are probably more, but until you straighten those out (especially the indentation), trying to figure out more is probably a waste of time.

Answer 3

c=s[i]!='\n' isn't doing what you think it's doing. The inequality operator has higher precedence than the assignment operator. The variable c will be set to one if s[i] isn't equal to '\n', and will be set to zero if it is equal.

You should either move the assignment to an earlier statement, or put parentheses around it.

Answer 4

I think you should at least attempt to read http://www.acm.uiuc.edu/webmonkeys/book/c_guide/introduction.html or something similar before attempting to solve your current problem. It seems you have absolutely no idea what you are doing, after you've had at least a bit of understanding of C the solution should be trivial.