Reputation: 21

I keep Getting Control reaches end of non-void function. What am I doing wrong?

location pick(void){    // generates a random location
     location get;
     get.x = rand() % FIELD_SIZE + 1;
     int forY = rand() % FIELD_SIZE +1;
    switch(forY){
    case 1:
        get.y = 'a';
            break;
    case 2:
        get.y = 'b';
            break;
    case 3:
        get.y = 'c';
            break;
    case 4:
        get.y = 'd';
            break;
    case 5:
        get.y = 'e';
            break;
    }

}

Upvotes: 2

Answers (6)

Jerry Coffin

Reputation: 490623

Hmm...now that you've been told how to fix the problem, here's how I'd write the code:

location pick(void){    // generates a random location
     location get;
     get.x = rand() % FIELD_SIZE + 1;
     get.y = rand() % FIELD_SIZE +'a';
     return get;
}

Purely in theory, this isn't entirely portable -- letters aren't required to be contiguous. In reality, the only one known where they aren't contiguous is EBCDIC, and it is contiguous in the range you're using. If you were really concerned about it, however, you could do something like:

location pick(void){    // generates a random location
     static char letters[] = "abcdef";
     location get;
     get.x = rand() % FIELD_SIZE + 1;
     get.y = letters[rand() % FIELD_SIZE];
     return get;
}

Upvotes: 0

Thomas Matthews

Reputation: 57749

You may want to pass the location item by reference to the function. This may reduce the stack size:

void pick(location& get)
{
     get.x = rand() % FIELD_SIZE + 1;
     int forY = rand() % FIELD_SIZE +1;
    switch(forY){
    case 1:
        get.y = 'a';
            break;
    case 2:
        get.y = 'b';
            break;
    case 3:
        get.y = 'c';
            break;
    case 4:
        get.y = 'd';
            break;
    case 5:
        get.y = 'e';
            break;
    }
    return;
}

Also, think about a default case in your switch statement. Error handling now will help prevent wasting later debugging time.

Another suggestion: convert switch statement into a table lookup. This allows the table to change without having to change the code (and retest the function). This can be further extended to placing the data into an external file. The external file allows changes to the data without the need to rebuild the program.

Upvotes: 0

Alessandra Sierra

Reputation: 10887

Your function, as written, does not return anything, but it is declared to return a location. You probably want return get; at the end, as Mark said.

Upvotes: 1