views:

42

answers:

2

I'm attempting to write an iPhone game. This function is intended to apply gravitational force to several objects. I'm porting it from Python and I'm wondering if my use of dictionaries and arrays as tuples makes sense and is typical/idiomatic in Objective C. Any comments on the code appreciated.

+ (void)updateBodies:(NSMutableArray*)bodies {
    NSMutableDictionary* totals = [NSMutableDictionary dictionaryWithCapacity:[bodies count]];
    for (Body* body in bodies) {
        if (body.fixed) {
            continue;
        }
        float tx;
        float ty;
        for (Body* other in bodies) {
            if (other == body) {
                continue;
            }
            float dx = other.x - body.x;
            float dy = other.y - body.y;
            float dist2 = pow(dx, 2) + pow(dy, 2);
            float dist = sqrt(dist2);
            float mass = pow(other.radius, 3);
            float magnitude = G * mass / dist2;
            float ux = dx / dist;
            float uy = dy / dist;
            tx += ux * magnitude;
            ty += uy * magnitude;
        }
        NSNumber* ntx = [NSNumber numberWithFloat:tx];
        NSNumber* nty = [NSNumber numberWithFloat:ty];
        NSArray* tuple = [NSArray arrayWithObjects:ntx, nty, nil];
        [totals setObject:tuple forKey:body];
    }
    for (Body* body in [totals allKeys]) {
        NSArray* tuple = [totals objectForKey:body];
        float tx = [[tuple objectAtIndex:0] floatValue];
        float ty = [[tuple objectAtIndex:1] floatValue];
        body.dx += tx;
        body.dy += ty;
    }
}
+1  A: 

You could used block enumeration for final update:

[totals enumerateKeysAndObjectsUsingBlock:^(id key, id obj, BOOL *stop) {
  Body* body = key;
  NSArray* tuple = key;
  body.dx += [[tuple objectAtIndex:0] floatValue];
  body.dy += [[tuple objectAtIndex:1] floatValue];
}];

An other solution could be to not used NSDictionary and NSArray and use a C array. It should be faster than using (and create) objects.

Benoît
He already is using fast enumeration. That's what `for(... in ...)` is.
JeremyP
Yes... I make a mistake. I would say block enumeration to not having a callto -allKeys and then objectForKey that should be costly than blocks
Benoît
+1  A: 

The only problem you should be aware of is that NSDictionary copies its keys. So Body needs to implement NSCopying and the instances of Body in totals are not necessarily the same instances in the passed in bodies array depending on how you implement NSCopying.

The approach I would use would be to consider velocity as a property of the body. That way you don't need a dictionary to associate the body to its velocity, you can just iterate through the array itself.


Talking of iterating. You can halve the number of iterations and some calculations by calculating the velocity of the other body at the same time as the first body. i.e. your inner loop would only iterate through the bodies that come after the outer loop body in the array.

It would mean you can't use fast iteration, so you'd have to profile to figure out which approach is faster.


On a minor note, I think

 for ....
 {
     if (!condition)
     {
         continue;
     }
     // do stuff
 }

is really ugly. What's wrong with:

 for ....
 {
     if (condition)
     {
         // do stuff
     }
 }
JeremyP
The `continue` approach reduces nesting. Just like an early return in a function.
FogleBird
Thanks, I ended up discovering the `NSCopying` issue. I switched to using an NSMutableArray instead of a NSMutableDictionary.
FogleBird
@FogleBird: It doesn't reduce nesting, it merely disguises it.
JeremyP
@JeremyP: What's your definition of nesting? It's the difference between `// do stuff` being one indent or two indents.
FogleBird
@FogleBird: The indentation of the source code should reflect its structure. The structure of both my examples is effectively the same seeing as we have a conditionally executed sequence of statements (the do stuff part) nested in a loop, but in the second, the indentation correctly reflects the structure.
JeremyP
Anyway, as I said, it's a minor point. You don't work for me so you don't have to code my way :)
JeremyP