up vote 2 down vote favorite
share [fb]

I want to generate random numbers within a range and the generated numbers should not collide within some range.
i used following code but im getting Stackoverflow error.. Any better solution?

static int [] xPositions=new int[10];
int WIDTH=700
public static void main(String args[])throws IOException
{

    if(generateRandomXPositions(10)){
        for(int i=0;i<10;i++){
          System.out.println(" Random Numbers "+i+"  :"+xPositions[i]);
        }
    }


}

private static boolean generateRandomXPositions(int n) {

    for(int i=0;i<10;i++){
        int temp=((int)0 + (int)(Math.random() * ((WIDTH - 0) + 1)));
        for(int j=0;j<xPositions.length;j++){
            if(xPositions[j]>temp-50 && xPositions[j]<temp+50){ // IF GENERATED NUMBER IS IN THIS RANGE IT SHOULD REGENERATE THE NUMBERS 
                generateRandomXPositions(10); 
            }
        }
        xPositions[i]=temp;
    }
    return true;
}

I know problem is here 

if(xPositions[j]>temp-50 && xPositions[j]<temp+50).

Below one works fine 

`if(xPositions[j]==temp)`.

But I need that random numbers should follow that range! .

Many are wondering about the exit condition of recursive loop. But I believe if random number is not in that range, then there is no point of entering in to the recursive loop.

UPDATE 1:

And I believe compiler is tired to find the number between this range! Now I found that it is impossible fit 10 images having width of 100px each in to the 700px width container without colliding X positions!

Please see the image below. Lets imagine i want to place this boxes randomly without colliding... how can i do that?

enter image description here

link|improve this question
82% accept rate
2  
it seems your recursive function is calling itself infinitely. check your if-condition! – clamp Oct 3 at 9:38
1  
if(xPositions[j]>temp && xPositions[j]<temp) will always evaluate to false. – S.L. Barth Oct 3 at 9:45
1  
@clamp,@ S.L. Barth if the random number is not fall in that range then there is no point of entering in to the recursive loop. – vnshetty Oct 3 at 9:55
What are you trying to do? And, why are you regenerating all members of xPositions? I think your program would have better performance if you used this line: for(int j=i+1 ;j<xPositions.length;j++) - but I'm not sure if it has the same semantics. – S.L. Barth Oct 3 at 10:04
If you want your boxes to be positioned randomly in both x and y, you'll want to rewrite your code. – skyuzo Oct 3 at 17:38
feedback

5 Answers

up vote 1 down vote accepted

You're getting a StackOverflowError because the chances of xPositions[j]>temp-50 && xPositions[j]<temp+50 not passing is very low when there's a range of 50. The chances of this function terminating is even lower due to the inner for-loop. Thus, this will keep recursing..

However, it doesn't seem like you're doing what you actually want to accomplish. If you want to generate numbers that are all within a set range, you don't want to compare xPositions[j] to temp-50 and temp+50. That's going to regenerate numbers when xPositions[j] isn't within some random range.

If you really just want to generate numbers that are within a certain range, then you'll want to get rid of the inner for-loop and instead do something like this:

for every number in xPositions:
  a = random number
  if a is within an unwanted range, regenerate numbers
  else set xPositions[i] = a

or without recursion:

for every number in xPositions:
  a = random number
  while a is within an unwanted range:
    a = random number
  set xPositions[i] = a

On the other hand, if you want to randomize the order of some images along the x-axis, you can do something like this:

bag = [0 1 2 ... n-1]
shuffle bag
for every number in xPositions:
  xPositions[i] = bag.pop * IMAGE_WIDTH
link|improve this answer
feedback
up vote 4 down vote

Since I think this is homework and your question is pretty vague, try to fill in these methods yourself and combine them intelligent.

public int generateRandom();
public boolean isAccepted(int number);
public void generate();

For generate(), use some loop like:

int temp = generateRandom();
while (!isAccepted(temp)) temp = generateRandom();
link|improve this answer
feedback
up vote 0 down vote

You've committed the classic error when writing recursive methods: no stopping condition.

Your method has to have one condition that returns false.

link|improve this answer
You're not thinking properly about this method at all. The range isn't important. I'll ask the question again: when will that method return false? Until you have a way to determine when the recursion ends you'll have this problem. – duffymo Oct 3 at 9:45
if the random number is not fall in that range then there is no point of entering in to the recursive loop. – vnshetty Oct 3 at 9:54
What's the point of using recursion at all? You're still not getting it. – duffymo Oct 3 at 10:01
feedback
up vote 0 down vote

Instead of calling generateRandomXPositions() again you should use a loop for creating a number and checking if it already exists or not.

The stackOverFlowError occurs because you recursively call the function from within itself.

link|improve this answer
feedback
up vote 0 down vote

You called generateRandomXPositions(10); inside a loop? This is inifinite loop, man. And you never use the param from the method.

link|improve this answer
if the random number is not fall in that range then there is no point of entering in to the recursive loop. – vnshetty Oct 3 at 9:59
feedback

Your Answer

 
or
required, but never shown

Not the answer you're looking for? Browse other questions tagged or ask your own question.