Managing People in a SimCity Clone

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 

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

@interface PCGameScene : SKScene 

@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

@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

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) BO

StackExchange Code Review Q#48445, answer score: 2