views:

128

answers:

6

Every now and again, the following comes out as -1

int randomIndex = random() % [directionsArray count] - 1;

The init method for this class is where directionsArray is created and srand() is called. It looks like this

- (id) initSprite
{
    if ( ( self = [super initSprite] ) ) {

        speed = 1;

        directionsArray = [[NSArray arrayWithObjects:STRING_CONSTANT, STRING_CONSTANT, nil] retain];
        srand(time(NULL));
        [self pickRandomDirection];


    }
    return self;
}

- (void) pickRandomDirection
{
    int randomIndex = random() % [directionsArray count] - 1; // This sometimes comes out as -1??
}

At the moment, I'm working around it by using abs(randomIndex), but that's cheating and I should probably know what's going on for future reference.

+1  A: 

Don't use random

Use arc4random instead.

Now that issue is solved, you're subtracting 1 from the randomization expression, which, in the case of arc4random % len returning 0, you'd be subtracting 1 from that, which is -1.

Jacob Relkin
In some cases, using `arc4random` instead of `random` is a good idea. In others, it makes no sense whatsoever. Picking a “random” direction for a sprite will generally fall in the latter category. However, the OP should be using `srandom` rather than `srand` if he’s sticking with `random`.
Ahruman
+1  A: 

I'd recommend arc4random. It seems to be the easiest to use and best rnd available in the SDK.

arc4random() % [directionsArray count];
joelm
+4  A: 

Does % have precedence over -1? If it does then when the remainder is 0 minuses 1 will result in negative one.

Jonathan Fischoff
+6  A: 

Because 0 - 1 equals -1.

    random() % [directionsArray count] - 1;

What if random() returns 0 here ? You get 0%[directionsArray count] - 1; which is -1, since % has precedence over -

Perhaps you want random() % ([directionsArray count] - 1);

nos
All great answers, but this is the one that made me go "oh right... of course it does..."
gargantaun
You probably should accept the answer then :)
Jeff Davis
had to wait 8 minutes.
gargantaun
+2  A: 

Know your operator precedence. If in doubt, use parentheses:

int randomIndex = random() % ([directionsArray count] - 1);
Stephen Canon
+2  A: 

I don't know why nobody else has mentioned this, but the solution (in your case) is actually to not subtract at all.

As you're probably aware, indexes are zero-based. Remainders likewise start at 0, and they go up to the divisor minus one and then loop around. Consider, for example, the case of count == 3:

  • 0 % 3 = 0
  • 1 % 3 = 1
  • 2 % 3 = 2
  • 3 % 3 = 0

Doing the division first, as Stephen Canon and Jonathan Fischoff already told you, will cause you to subtract 1 from these valid indexes, causing the problem you saw.

Doing the division second is not much better: It solves the problem of coming up with an index of -1, but you're still cutting off the last possible index:

  • 0 % (count - 1 = 2) = 0
  • 1 % 2 = 1
  • 2 % 2 = 0
  • 3 % 2 = 1

Notice how index 3 never comes up.

Divide only; cut out the subtraction.

Also, indexes are normally NSUInteger (an unsigned integral type, unlike int, which is signed) in Cocoa and Cocoa Touch.

Peter Hosey
This answer should be up higher, as it actually codes what the asker intended.
BarrettJ