Take the 2-minute tour ×

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!=' ')) {
            if ( (c<'0') || (c>'9') ) {
                return(-1); //was exit testing return, **this always kicks me out**
                if( counter > 6){
                } //was exit testing return
                value = value*10+'c'-'0';
        if ((c=getchar())==' ') {      
    return (counter);
    printf("%c,%c:",counter,value); // for testing
share|improve this question
Arrrrgggghhhh! Indent your code properly. –  pmg Sep 28 '10 at 23:56
I am sorry, I haven't learned the proper way to indent yet. Our instructor said he would send us a guideline sometime this week. –  pisfire Sep 29 '10 at 0:02
The code is either too messy, to spot the initialization of j or there is no initialization of j. In any case it's bad. –  Maciej Hehl Sep 29 '10 at 0:03
You need to check the sizeof(numarray) before adding to it in case you run off the end (buffer overflow). I assume you intend to put {} around numarray[j]=value; j++; counter++; –  MattSmith Sep 29 '10 at 0:05
@MattSmith sizeof(numarray) == sizeof(int*) inside this function. The size would have to be passed as a parameter. –  Maciej Hehl Sep 29 '10 at 0:10
show 2 more comments

4 Answers

up vote 7 down vote accepted

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.

share|improve this answer
ok thank you very much –  pisfire Sep 29 '10 at 0:04
add comment

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.

share|improve this answer
ok thank you very much –  pisfire Sep 29 '10 at 0:03
add comment

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?

share|improve this answer
+1 for last sentence especially –  alex Sep 28 '10 at 23:58
add comment

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.

share|improve this answer
Yeah I agree. First week of class was "hello World" our first programming assignment that we actually write is suppose to take any square matrix from 2x2 to 6x6 and spit out the answer. I do not know how I am going to get this done or keep my sanity. I am 16 hours in on this assignment and way way over my head. Half the responses I get on here; I do not even know they mean. –  pisfire Sep 29 '10 at 1:10
add comment

Your Answer


