tags:

views:

124

answers:

6
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;
    }

}
+9  A: 

You're missing return get; at the end of your function.

Mark Byers
HA, wow thank you.
Allen
@Allen: Click the green check mark to indicate your question is answered.
GMan
+1  A: 

Are you returning a value from your function?

Brian R. Bondy
+6  A: 

Make sure to return get; in your function!

Otherwise, you want to declare your function as void pick.

Justin Ardini
+1  A: 

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.

Stuart Sierra
A: 

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.

Thomas Matthews
A: 

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;
}
Jerry Coffin