0

I'm having trouble getting random numbers to generate. For some reason I'm getting huge, and some negative, numbers when I print the random numbers after the while loop. No idea what's going on so any help is greatly appreciated!

int genRan(int numHigh, int numLow){

    int ranNum = rand() % (numHigh) + numLow;
    return ranNum;

}

generateDungeon(){
    int ranRow, ranCol, ranWidth, ranHeight;
    for(int i = 0; i<5; i++){
        bool check = false;
        while(check == false){
            int ranRow = genRan(18, 1);
            int ranCol = genRan(77,1);
            int ranWidth = genRan(8,3);
            int ranHeight = genRan(7,2);
            if((ranCol + ranWidth) <= 78 && (ranRow + ranHeight) <= 19){
                check = true;
            }

        }
        printf("%d\n", ranRow);
        printf("%d\n", ranCol);
        printf("%d\n", ranWidth);
        printf("%d\n\n\n", ranHeight);

        for(int x = ranCol; x<ranWidth + ranCol; x++){
            for(int y = ranRow; y<ranHeight + ranRow; y++){
                //dungeon[x][y] = '.';
            }
            //printf("%d ", x);
        }
   }
}

int main(){
    srand(time(0));
    generateDungeon();
    //genRan();
    return 0;
}
Stefan
  • 27,908
  • 4
  • 53
  • 82
Mitchell Kerr
  • 1
  • 1
  • Please see [this answer](https://stackoverflow.com/a/1202706/4142924) about generating random numbers within a range. – Weather Vane Jan 21 '18 at 00:23
  • `generateDungeon` should be declared as `void generateDungeon()`. – Eric Postpischil Jan 21 '18 at 00:25
  • 1
    It is not clear whether you want `ranNum = rand() % (numHigh) + numLow` or `ranNum = rand() % (numHigh-numLow) + numLow` (or possibly `numHigh+1-numLow`). Also `%` is a bad way to reduce the range of `rand`, because historic implementations of `rand` are bad and have low “randomness” in their low bits. – Eric Postpischil Jan 21 '18 at 00:26
  • If the errors persist after you correct the range problem above and the initialization problem below, provide a [minimal complete verifiable example](https://stackoverflow.com/help/mcve). It should include all the headers (`` and so on) needed to compile, should seed `srand` with a constant rather than the time (so that other people can reproduce **exactly**), and should report the output you observe and the output you want. (`rand` may continue to be a problem with a fixed seed, as different C implementations may use different `rand` implementations. That can be dealt with.) – Eric Postpischil Jan 21 '18 at 00:29
  • Inside the `while` loop, you have `int ranRow = genRan(18, 1);`. This declares a **new** `ranRow` that is different from the one outside the loop. The `ranRow` outside the loop remains unchanged and hence uninitialized. The resulting behavior is undefined. Remove `int` from the statements inside the loop, so they are simple assignments, not definitions. – Eric Postpischil Jan 21 '18 at 00:31
  • You should have debugged this code by inspecting the values at various points in the `generateDungeon` routine, either by using a debugger or by inserting `printf` statements. Examining the values at various points would reveal where things go wrong and brought your attention to the fact that `ranRow` and the other objects do not have the expected values after the `while` loop terminates. – Eric Postpischil Jan 21 '18 at 00:40
  • @user3629249: `%` has higher precedence than `+`. Enclosing `%` in parentheses does not change the expression. – Eric Postpischil Jan 21 '18 at 04:06
  • regarding: `int ranNum = rand() % (numHigh) + numLow;` Since you want the result to be in the range `numLow` ... `numHigh` This code will produce the result in the range `numLow` ... (`numHigh` + `numLow`) Suggest: `int ranNum = rand() % (numHigh-numLow) + numLow;` – user3629249 Jan 22 '18 at 15:32

1 Answers1

1

This code:

int ranRow, ranCol, ranWidth, ranHeight;
    for(int i = 0; i<5; i++){
        bool check = false;
        while(check == false){
            int ranRow = genRan(18, 1);
            int ranCol = genRan(77,1);
            int ranWidth = genRan(8,3);
            int ranHeight = genRan(7,2);
            if((ranCol + ranWidth) <= 78 && (ranRow + ranHeight) <= 19){
                check = true;
            }

defines new objects ranRow, ranCol, ranWidth, and ranHeight inside the loop. These new definitions hide the previous ones outside the loop, and the objects outside the loop are not changed. This results in them not being assigned values, so later uses of them have undefined behavior. To fix this, remove the int keywords from the statements inside the loop, so they are simple assignments rather than definitions.

(Your compiler should have warned you about the use of uninitialized objects. If it did not, look for ways to turn on more warnings and use them by default.)

In this code:

int ranNum = rand() % (numHigh) + numLow;

it is not clear whether you actually want (numHigh). You might want (numHigh-numLow) or (numHigh-numLow+1).

More astute use of ranges in genRan could eliminate the use of the while loop to set the initial values.

If generateDungeon is not intended to return a value, it should be declared with:

void generateDungeon()
Eric Postpischil
  • 195,579
  • 13
  • 168
  • 312
  • Helpful link: [Get warning when a variable is shadowed](https://stackoverflow.com/questions/25151524/get-warning-when-a-variable-is-shadowed) with info on gcc, and link to VS options. – David C. Rankin Jan 21 '18 at 03:30
  • Thank you, wow I should have noticed I was declaring the variables twice. Thanks for your help! – Mitchell Kerr Jan 21 '18 at 17:57