5
\$\begingroup\$

So I am working through a few different game ideas as I learn programming, and I could definitely use some feedback on my latest project. This is a simple SimCity clone. So far I have created a City made up of City Tiles objects, and a number of Person objects as well. The Persons are passed from the City Tiles to the City and then on to the next City Tile, so each Tile has a certain number of Persons inside of it. Also different terrain types take longer for Persons to move through. The renderer changes the alpha of the sprite that it draws for the Tile based on the number of persons inside. Right now things just move in a random fashion, but later the Game would assign destination tiles to the Persons based on what was happening in the game.

My main concern is firstly whether or not the way I am doing things makes any sense. I am also concerned with the readability of the code, and following Objective-C best practices.

First here is the SKScene class, with the necessary panning and zooming gestures configured. PCGameScene.h:

#import <SpriteKit/SpriteKit.h>

//set up for 60 frames per second
#define kMinTimeInterval (1.0f / 60.0f)

@interface PCGameScene : SKScene <UIGestureRecognizerDelegate>

@property (nonatomic) NSTimeInterval lastUpdateTimeInterval;
@property (nonatomic) NSTimeInterval countdownInterval;
@property (nonatomic) NSTimeInterval countdownIntervalRender;

@end

PCGameScene.m

#import "PCGameScene.h"
#import "PCGame.h"
#import "PCCityTile.h"
#import <GLKit/GLKit.h>

@implementation PCGameScene {
    //initialization
    PCGame *_game;
    CGSize _initialScreenSize;
    BOOL _contentCreated;

    //scene components
    SKNode *_world;
    SKNode *_sceneCity;

    //hud components
    SKNode *_selectedNode;
    int _selectedFloor;

    //gesture components
    UIPinchGestureRecognizer *_pinchRecognizer;
    UIPanGestureRecognizer *_panRecognizer;
    UISwipeGestureRecognizer *_swipeLeft;
    UISwipeGestureRecognizer *_swipeRight;
    CGPoint _startingButtonLocation;
    CGPoint _previousTouchPoint;
}

#pragma mark - Initialization
-(void) didMoveToView:(SKView *)view {
    [self createUIAndRenderer];
    [self createSceneElements];
    if (!_contentCreated) {
        [self createSceneContents];
        _contentCreated = YES;
    } 
}
-(void) createUIAndRenderer {
    _initialScreenSize = self.size;

    //set up the camera
    _world = [[SKNode alloc]init];
    [self addChild:_world];
    _world.position = CGPointMake(_initialScreenSize.width/2, 0);

    //set up the backgrounds
    self.backgroundColor = [SKColor whiteColor];

    //Set up the gesture recognizers
    _panRecognizer = [[UIPanGestureRecognizer alloc]initWithTarget:self action:@selector(handlePanFrom:)];
    [[self view] addGestureRecognizer:_panRecognizer];
    _pinchRecognizer = [[UIPinchGestureRecognizer alloc]initWithTarget:self action:@selector(handlePinch:)];
    [[self view] addGestureRecognizer:_pinchRecognizer];
    _panRecognizer.delegate = self;
    _pinchRecognizer.delegate = self;
    _panRecognizer.maximumNumberOfTouches = 1;

    //Set up the texture atlases here later

    //create the hud
    [self buildHud];
}
-(void) createSceneElements {
    _sceneCity = [[SKNode alloc]init];
    [_world addChild:_sceneCity];
}
-(void) createSceneContents {
    _game = [[PCGame alloc]init];
}

#pragma mark - Update Loop
- (void)update:(NSTimeInterval)currentTime {
    // Handle time delta.
    // If we drop below 60fps, we still want everything to move the same distance.
    CFTimeInterval timeSinceLast = currentTime - self.lastUpdateTimeInterval;

    //update game and hud counters every quarter second
    self.countdownInterval += timeSinceLast;
    if (self.countdownInterval > 0.25) {
        [_game  updateCity];
        self.countdownInterval = 0;
    }

    //render more frequently than the game is updated
    self.countdownIntervalRender += timeSinceLast;
    if (self.countdownIntervalRender > 0.10) {
        [self renderCity];
        self.countdownIntervalRender = 0;
    }

    self.lastUpdateTimeInterval = currentTime;
    if (timeSinceLast > 1) { // more than a second since last update
        //timeSinceLast = kMinTimeInterval;
        self.lastUpdateTimeInterval = currentTime;
    }
    //[self updateWithTimeSinceLastUpdate:timeSinceLast];
}

#pragma mark - Rendering
-(void) renderCity {
    [_sceneCity removeAllChildren];
    NSArray *tempArray = [_game getChangedTilesForRender];
    for (PCCityTile *tempTile in tempArray) {
        SKSpriteNode *cityTile = [[SKSpriteNode alloc]init];
        cityTile.size = CGSizeMake(1,1);
        cityTile.position = tempTile.position;

        //the alpha of the tile is based on how many persons are inside it
        //multiply it by .30 to make it a little darker than just 0.10
        CGFloat tileAlpha = tempTile.personArray.count * 0.30;
        cityTile.color = [SKColor colorWithRed:0.00 green:0.00 blue:0.00 alpha:tileAlpha];

        [_sceneCity addChild:cityTile];
    }
}

#pragma mark - Gesture Handling
-(void) handlePanFrom:(UIGestureRecognizer *)recognizer {
    CGPoint touchLocation = [recognizer locationInView:recognizer.view];
    touchLocation = [self convertPointFromView:touchLocation];
    if (recognizer.state == UIGestureRecognizerStateBegan) {
        _previousTouchPoint = touchLocation;
    } else if (recognizer.state == UIGestureRecognizerStateChanged) {
        CGPoint tempPosition = CGPointAdd(_world.position, CGPointSubtract(touchLocation, _previousTouchPoint));
        _world.position = CGPointMake(tempPosition.x, tempPosition.y);
        _previousTouchPoint = touchLocation;
    } else if (recognizer.state == UIGestureRecognizerStateEnded) {
    }
}
- (void)handlePinch:(UIPinchGestureRecognizer *)recognizer {
    CGPoint touchLocation = [recognizer locationInView:recognizer.view];
    touchLocation = [self convertPointFromView:touchLocation];
    if (recognizer.state == UIGestureRecognizerStateBegan) {
        // No code needed for zooming
    } else if (recognizer.state == UIGestureRecognizerStateChanged) {
        CGPoint anchorPoint = CGPointSubtract(touchLocation, _world.position);
        CGPoint mySkNodeShift = CGPointSubtract(anchorPoint, CGPointMultiplyScalar(anchorPoint, recognizer.scale));

        [_world runAction:[SKAction group:@[
                                            [SKAction scaleBy:recognizer.scale duration:0.0],
                                            [SKAction moveBy:CGVectorMake(mySkNodeShift.x, mySkNodeShift.y) duration:0.0]
                                            ]]];
        recognizer.scale = 1.0;
        } else if (recognizer.state == UIGestureRecognizerStateEnded) {
            // No code needed here for zooming
        }
    }

#pragma mark - Touch Handling
-(void)touchesEnded:(NSSet *)touches withEvent:(UIEvent *)event {
    for (UITouch *touch in touches) {
        if(touch.tapCount == 1) {
            CGPoint positionInScene = [touch locationInNode:self];
            [self selectNodeForTouch:positionInScene];
        }
    }
}
-(void)selectNodeForTouch:(CGPoint)touchLocation {
    //we need the touched node
    SKSpriteNode *touchedNode = (SKSpriteNode *)[self nodeAtPoint:touchLocation];
    _selectedNode = touchedNode;
    //NSLog(@"node name is = %@", touchedNode.name);

    //touchLocation = [self convertTouchPointToWorld:touchLocation];

    //handle the main hud buttons
    if ([touchedNode.name isEqualToString:@"pauseButton"]) {
    }
    if ([touchedNode.name isEqualToString:@"menuButton"]) {
    }
}

#pragma mark - HUD
-(void) buildHud {
    SKSpriteNode *pauseButton = [[SKSpriteNode alloc]initWithColor:[SKColor redColor] size:CGSizeMake(_initialScreenSize.width/6, _initialScreenSize.height/10)];
    pauseButton.name = @"pauseButton";
    pauseButton.position = CGPointMake(_initialScreenSize.width - _initialScreenSize.width/10, _initialScreenSize.height/12);
    [self addChild:pauseButton];

    SKSpriteNode *menuButton = [[SKSpriteNode alloc]initWithColor:[SKColor greenColor] size:CGSizeMake(_initialScreenSize.width/6, _initialScreenSize.height/10)];
    menuButton.name = @"menuButton";
    menuButton.position = CGPointMake(_initialScreenSize.width - _initialScreenSize.width/3, _initialScreenSize.height/12);
    [self addChild:menuButton];
}

#pragma mark - Helper functions
static inline CGPoint CGPointAdd(CGPoint point1, CGPoint point2) {
    return CGPointMake(point1.x + point2.x, point1.y + point2.y);
}
static inline CGPoint CGPointSubtract(CGPoint point1, CGPoint point2) {
    return CGPointMake(point1.x - point2.x, point1.y - point2.y);
}
static inline GLKVector2 GLKVector2FromCGPoint(CGPoint point) {
    return GLKVector2Make(point.x, point.y);
}
static inline CGPoint CGPointFromGLKVector2(GLKVector2 vector) {
    return CGPointMake(vector.x, vector.y);
}
static inline CGPoint CGPointMultiplyScalar(CGPoint point, CGFloat value) {
    return CGPointFromGLKVector2(GLKVector2MultiplyScalar(GLKVector2FromCGPoint(point), value));
}
- (BOOL)gestureRecognizer:(UIGestureRecognizer *)gestureRecognizer shouldRecognizeSimultaneouslyWithGestureRecognizer:(UIGestureRecognizer *)otherGestureRecognizer {
    return YES;
}
-(CGPoint) convertTouchPointToWorld:(CGPoint)touchLocation {
    CGPoint locationInWorld = [self.scene convertPoint:touchLocation toNode:_world];
    return locationInWorld;
}

@end

Here is the Game class, doesn't do too much yet.
PCGame.h:

#import <Foundation/Foundation.h>

@interface PCGame : NSObject

-(void) updateCity;
-(NSMutableArray *) getChangedTilesForRender;

@end

PCGame.m:

#import "PCGame.h"
#import "PCCity.h"

@implementation PCGame {
    PCCity *_city;
    NSArray *_gameChangedTiles;
}

#pragma mark - Initialization
-(id) init {
    self = [super init];
    if (self) {
    _city = [[PCCity alloc]init];
    }
    return self;
}

#pragma mark - Update loop 
-(void) updateCity {
    [_city updateCity];
    [self updateChangedTiles];
}
-(void) updateChangedTiles {
    _gameChangedTiles = _city.changedTiles;
}

#pragma mark - Interface with Renderer
-(NSArray *) getChangedTilesForRender {
    return _gameChangedTiles;
}

@end

Here is the City class, this is the class that ends up determining which City Tile to pass a Person on to. PCCity.h:

#import <Foundation/Foundation.h>

@interface PCCity : NSObject

-(void) updateCity;
@property NSMutableArray *changedTiles;

@end

PCCity.m:

#import "PCCity.h"
#import "PCCityTile.h"
#import "PCPerson.h"
#import "PCPersonReporter.h"

#define kMapWidth 100
#define kMapHeight 100

@implementation PCCity {
    NSMutableArray *_personArray;
    NSMutableDictionary *_cityTiles;
}

-(id) init {
    self = [super init];
    if (self) {
        _cityTiles = [[NSMutableDictionary alloc]init];
        _personArray = [[NSMutableArray alloc]init];
        self.changedTiles = [[NSMutableArray alloc]init];

        [self createCity];
        [self createPeople];
        [self sendPeopleToTiles];
    }
    return self;
}
-(void) createCity {
    NSMutableDictionary *tempDictionary = _cityTiles;

    //how many tiles to create at the start
    int mapWidth = kMapWidth;
    int mapHeight = kMapHeight;
    int mapStartX = 0;
    int mapStartY = 0;

    int tileX = mapStartX;
    int tileY = mapStartY;
    while (tileX < mapWidth) {
        while (tileY < mapHeight) {
            PCCityTile *tempTile = [[PCCityTile alloc]init];
            tempTile.position = CGPointMake(tileX,tileY);
            NSString *tempTilePosition = NSStringFromCGPoint(tempTile.position);
            [tempDictionary setObject:tempTile forKey:tempTilePosition];
            tileY++;
        }
        tileX++;
        tileY = mapStartY;
    }
    _cityTiles = tempDictionary;
}
-(void) createPeople {
    for (int i = 0; i < 500; i++) {
        int randomStartX = arc4random_uniform(kMapWidth);
        int randomStartY = arc4random_uniform(kMapHeight);
        PCPerson *tempPerson = [[PCPerson alloc]init];
        tempPerson.currentTile = CGPointMake(randomStartX, randomStartY);
        //having the destination tile be the same as the current tile will cause a person to pick a new random one
        tempPerson.destinationTile = CGPointMake(randomStartX, randomStartY);
        [_personArray addObject:tempPerson];
    }
    //the reporter is a regular person except it NSLogs certain information
    PCPersonReporter *reporter = [[PCPersonReporter alloc]init];
    reporter.currentTile = CGPointMake(50, 50);
    reporter.destinationTile = CGPointMake(20, 20);
    [_personArray addObject:reporter];
}
-(void) sendPeopleToTiles {
    for (PCPerson *tempPerson in _personArray) {
        NSString *sendToTileString = NSStringFromCGPoint(tempPerson.currentTile);
        PCCityTile *sendToTile = [_cityTiles objectForKey:sendToTileString];
        [sendToTile acceptPerson:tempPerson];
    }
    [_personArray removeAllObjects];
}

#pragma mark - Update loop 
-(void) updateCity {
    [self.changedTiles removeAllObjects];

    [self updateCityTiles];
    [self getPersonsFromTiles];
    [self movePersons];

    [self createChangedTileArray];
}
-(void) updateCityTiles {
    for (id key in _cityTiles) {
        PCCityTile *tempTile = [_cityTiles objectForKey:key];
        [tempTile updateCityTile];
    }
}
-(void) getPersonsFromTiles {
    for (id key in _cityTiles) {
        PCCityTile *tempTile = [_cityTiles objectForKey:key];
        if (tempTile.personsForPickup.count > 0) {
            for (PCPerson *person in tempTile.personsForPickup) {
                person.isInsideCity = YES;
                person.isInsideTile = NO;
                [_personArray addObject:person];
            }
            tempTile.tileHasChanged = YES;
            [tempTile.personsForPickup removeAllObjects];
        }
    }
}
-(void) movePersons {
    for (PCPerson *person in _personArray) {
        int currentXPos = person.currentTile.x;
        int currentYPos = person.currentTile.y;

        int destinationXPos = person.destinationTile.x;
        int destinationYPos = person.destinationTile.y;

        int xDifference = currentXPos - destinationXPos;
        int yDifference = currentYPos - destinationYPos;

        //moves in the x direction first and then the y if x has not moved
        BOOL hasAlreadyMoved = NO;
        if (xDifference != 0) {
            if (xDifference > 0) {
                currentXPos--;
                hasAlreadyMoved = YES;
            } else if (xDifference < 0) {
                currentXPos++;
                hasAlreadyMoved = YES;
            }
        }
        if (yDifference != 0 && !hasAlreadyMoved) {
            if (yDifference > 0) {
                currentYPos--;
            } else if (yDifference < 0) {
                currentYPos++;
            }
        }

        CGPoint moveToPoint = CGPointMake(currentXPos, currentYPos);
        NSString *moveToTileString = NSStringFromCGPoint(moveToPoint);
        PCCityTile *moveToTile = [_cityTiles objectForKey:moveToTileString];
        [moveToTile acceptPerson:person];

        moveToTile.tileHasChanged = YES;
    }
    [_personArray removeAllObjects];
}
-(void) createChangedTileArray {
    for (id key in _cityTiles) {
        PCCityTile *tempTile = [_cityTiles objectForKey:key];
        if (tempTile.tileHasChanged) {
            [self.changedTiles addObject:tempTile];
            tempTile.tileHasChanged = NO;
        }
    }
}

@end

Here is the City Tile class. PCCityTile.h:

#import <Foundation/Foundation.h>
#import "PCPerson.h"
#import "TerrainTypes.h"

@interface PCCityTile : NSObject

@property CGPoint position;
@property TerrainType terrainType;
@property NSMutableArray *personArray;
@property NSMutableArray *personsForPickup;

-(void) updateCityTile;
@property BOOL tileHasChanged;

-(void) acceptPerson:(PCPerson *)person;

@end

PCCityTile.m:

#import "PCCityTile.h"

@implementation PCCityTile

-(id) init   {
    self = [super init];
    if (self) {
        self.personArray = [[NSMutableArray alloc]init];
        self.personsForPickup = [[NSMutableArray alloc]init];
        self.terrainType = arc4random_uniform(7);
    }
    return self;
}

-(void) acceptPerson:(PCPerson *)person {
    person.currentTile = self.position;
    person.startCountForTerrain = self.terrainType;
    [person startCounting];

    person.isInsideTile = YES;
    person.isInsideCity = NO;

    [self.personArray addObject:person];
}

#pragma mark - Update loop
-(void) updateCityTile {
    [self checkPersons];
    [self setNewDestinations];
}
-(void) checkPersons {
    NSMutableArray *personsToKeep = [[NSMutableArray alloc]init];
    NSMutableArray *personsToPickup = [[NSMutableArray alloc]init];
    for (PCPerson *person in self.personArray) {
        [person checkStatus];
        if (!person.isAtDestination) {
            if (person.isTerrainCountComplete) {
                person.isBeingPickedUp = YES;
                person.isStayingAtTile = NO;
                [personsToPickup addObject:person];
            } else {
                person.isStayingAtTile = YES;
                person.isBeingPickedUp = NO;
                //have to do this here so the tiles show up even when persons are staying put
                self.tileHasChanged = YES;
                [personsToKeep addObject:person];
            }
        } else {
            person.isStayingAtTile = YES;
            person.isBeingPickedUp = NO;
            [personsToKeep addObject:person];
        }
    }
    [self.personsForPickup setArray:personsToPickup];
    [self.personArray setArray:personsToKeep];
}
-(void) setNewDestinations {
    for (PCPerson *person in self.personArray) {
        if (person.isAtDestination) {
            int randomX = arc4random_uniform(75);
            int randomY = arc4random_uniform(75);
            person.destinationTile = CGPointMake(randomX, randomY);
        }
    }
}

@end

Finally, the Person class itself. Person.h:

#import <Foundation/Foundation.h>
#import "TerrainTypes.h"

@interface PCPerson : NSObject

@property CGPoint currentTile;
@property CGPoint destinationTile;
@property TerrainType startCountForTerrain;
@property int currentTerrainCount;

@property BOOL isAtDestination;
@property BOOL isBeingPickedUp;
@property BOOL isStayingAtTile;
@property BOOL isInsideCity;
@property BOOL isInsideTile;
@property BOOL isTerrainCounting;
@property BOOL isTerrainCountComplete;

-(void) checkStatus;
-(void) sayStatus;
-(void) startCounting;

@end

Person.m:

#import "PCPerson.h"

@implementation PCPerson

-(void) checkStatus {
    if (self.currentTile.x == self.destinationTile.x && self.currentTile.y == self.destinationTile.y) {
        self.isAtDestination = YES;
    } else {
        self.isAtDestination = NO;
        if (self.isTerrainCounting) {
            [self doCountdown];
        }
    }
    [self sayStatus];
}
-(void) sayStatus {
    //overridden by subclasses
}
-(void) startCounting {
    self.isTerrainCounting = YES;
    self.isTerrainCountComplete = NO;
    switch (self.startCountForTerrain) {
        case Impassable:
            self.currentTerrainCount = 100;
            break;
        case Mountain:
            self.currentTerrainCount = 90;
            break;
        case Desert:
            self.currentTerrainCount = 80;
            break;
        case Swamp:
            self.currentTerrainCount = 70;
            break;
        case Forest:
            self.currentTerrainCount = 60;
            break;
        case Hills:
            self.currentTerrainCount = 50;
            break;
        case Grassland:
            self.currentTerrainCount = 40;
            break;
        default:
            break;
    }
}
-(void) doCountdown {
    if (self.currentTerrainCount > 0) {
        self.currentTerrainCount-=100;
    } else {
        self.isTerrainCounting = NO;
        self.isTerrainCountComplete = YES;
    }
}

@end
\$\endgroup\$
2
\$\begingroup\$

Note: I don't have time right out a complete answer, so you'll have to check back this afternoon, but I'll get one started.


First, the most glaring problem I see is your use of the self. accessors in the init methods. It seems to be a common problem here (I see it on SO a lot too), but Apple actually explicitly states that you should not be doing this.

You should be accessing the backing variable directly:

-(id) init {
    self = [super init];
    if (self) {
        _personArray = [NSMutableArray array];
        _personsForPickup = [NSMutableArray array];
        _terrainType = arc4random_uniform(7);
    }
    return self;
}

And while it's technically not incorrect, for me, an Objective-C class isn't complete without a public factory method for every public init method.

+ (instancetype)cityTyle {
    return [[self alloc] init];
}

Next, I think we need a little mini-lesson on properties and property attributes.

First of all, while getters in other languages are prefixed with getSomeVar, in Objective-C, they are not, and the accessor would just be called someVar.

So, at a minimum, -(NSMutableArray *) getChangedTilesForRender; should be changed to - (NSMutableArray *)changedTilesForRender;.

But the problem for me is the inconsistency between the City and Game class, where one has a property and the other has a method.

Let's look at the Game class first. If we change the method declaration to the following:

@property (nonatomic,strong,readonly) NSMutableArray *changedTilesForRender;

Now we have created a getter and a backing instance variable. There is no setter, so the variable can't be changed by outside classes. What's more, the backing variable will by default be called _changedTilesForRender. This is a good thing.

If I'm maintaining this code, whether I didn't write it, or I did write it but it's been 6 months since I wrote it and I just now found a bug, I'm not going to know where all the information for a stand-alone getter method comes from. It might be hard to follow. But when we declare a read-only property, that has a well-defined backing variable.

If you don't want the backing variable to have the same name as the property, you can always @synthesize.

@synthesize changedTilesForRender = fooBarExampleName;

And this is still better, because when I Ctrl+F to find the former, it'll take me to a line that clearly redefines it as the latter, and now I can Ctrl+F through the code for that one.

Now, in the City class, changedTiles isn't marked as readonly. Is this intentional?


@property BOOL isAtDestination;
@property BOOL isBeingPickedUp;
@property BOOL isStayingAtTile;
@property BOOL isInsideCity;
@property BOOL isInsideTile;
@property BOOL isTerrainCounting;
@property BOOL isTerrainCountComplete;

This is a mess.

Any time you need 2 or more BOOLs to describe a single status, you might be better off with an enum.

Your class isn't complete, so I don't know the full meaning or intended use of all of these, but it looks like isTerrainCounting and isTerrainCountComplete are going to be mutually exclusive. The problem with using two BOOLs to describe this single status is that there's nothing you can do to actually make these mutually exclusive as written.

And with multithreading, it's not completely impossible that something else checks the status of PCPerson and come back with both YES or both NO. It may be rare and difficult but not impossible. What's more, this is prone to mistakes. Whether from you or someone else who might look at this code, nothing guarantees that the code will be correctly written to set both.

The only way to make these statuses guaranteed to be mutually exclusive is by making them a single variable. Simply eliminating one of them is an option, and if it's safe to assume that any object that isn't currently terrain counting is thereby count complete--that is to say, there's no status where the object hasn't completed or started counting.

But we can even improve the readability of this single BOOL:

typedef NS_ENUM(BOOL, TerrainCountStatus) {
    TerrainCountStatusCounting = YES,
    TerrainCountStatusCountComplete = NO
};

@property TerrainCountStatus terrainCountStatus;

Now you set it like this:

self.terrainCountStatus = TerrainCountStatusCounting;

But what might be even better is an enum that accounts for the third status possibility.

typedef NS_ENUM(NSInteger, TerrainCountStatus) {
    TerrainCountStatusNoCount = -1,
    TerrainCountStatusCounting = 0,
    TerrainCountStatusCountComplete = 1
}

Now you set the values in the same way as the previous example.

But now you can represent three statuses with a single variable and they are guaranteed to be mutually exclusive because it's only a single variable.

What's more, you can still keep the BOOL properties if you want to more easily check YES/NO on a single status.

@property (readonly) BOOL isTerrainCounting;

Don't forget the readonly, now override the accessor:

- (BOOL)isTerrainCounting {
    return (self.terrainCountStatus == TerrainCountStatusCounting);
}

You don't seem to have included TerrainTypes in this code review, but it seems it's probably an enum? Why not define the values to the starting values you're setting instead of switching and setting a value based on the terrain type?


#define kMinTimeInterval (1.0f / 60.0f)
#define kMapWidth 100
#define kMapHeight 100

These would all be better as const values.

const CGFloat kMinTimeInterval = (1.0f / 60.0f);
const NSInteger kMapWidth = 100;
const NSInteger kMapHeight = 100;

And the latter two could even be an enum instead of a const if you wanted, for example:

typedef NS_ENUM(NSInteger, MapDimensions) {
    kMapWidth = 100,
    kMapHeight = 100
};

#define is notoriously hard to debug. It's just preprocessor find & replace, so if there's any sort of bug with it, the compiler won't be too particularly helpful relative to using a const or enum.

The one advantage that #define would even potentially have over anything else is that because the preprocessor finds & replaces, your programming, during run time, might be ever so slightly more efficient. But this is extremely insignificant, and a good compiler will make this completely irrelevant in a lot of cases.

At the end of the day, there aren't a lot of good excuses for using #define.

\$\endgroup\$
  • 1
    \$\begingroup\$ Extremely helpful as always! Thank you. Based on some of the recommendations here I am writing radically different code than I was before, which is awesome. \$\endgroup\$ – bazola May 3 '14 at 17:09

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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