Game Achievements without Singleton

This is my first time implementing achievements in a game so I could definitely use some feedback on what I have done here. I have heard about singleton approaches to this problem, but I did not feel it was really necessary and I would rather avoid singletons since they have a bad reputation. Instead my approach is to store a custom object in NSUserDefaults and load it at the start of a game. One important aspect of achievements in my game is that having certain achievements unlocked will unlock various bonuses in the game, but these effects will only be applied at the start of a new game.

My first time working through trying to set this up, I tried to have a simple dictionary hold boolean values for whether the achievement was completed or not. Later I decided that it could be important to show the amount of progress completed on an achievement, so I implemented classes to handle them. With this way of doing it, the Game only needs to have an AchievementType to access the achievement and update its completion state.

DTAchievements.h

#import 
#import "DTAchievement.h"

@interface DTAchievements : NSObject 

-(void) completeAchievement:(AchievementType)type;

@property NSMutableDictionary *achievementsDictionary;

@end

DTAchievements.m

```
#import "DTAchievements.h"

NSString* const k100FloorsRevealed = @"100FloorsRevealed";
NSString* const k100DwarvesDead = @"100DwarvesDead";
NSString* const k5thTowerDiscovered = @"5thTowerDiscovered";
NSString* const kLeaderDiedOldAge = @"leaderDiedOldAge";
NSString* const kBottomFloorRevealed = @"bottomFloorRevealed";
NSString* const kTopFloorRevealed = @"topFloorRevealed";

NSDictionary* achievementsDict() {
NSDictionary *achievements = @{ k100FloorsRevealed :
[[DTAchievement alloc]initWithType:AchievementType100FloorsRevealed],
k100DwarvesDead :
[[DTAchievement alloc]initWithType:AchievementTy

As I commented, you really should look into using the GameCenter to handle all of your achievements. With that said, here's my thoughts on some of what you are doing.

It doesn't ever make sense to ever have more than one DTAchievements instantiated, does it? It also seems a little clunky that we have this DTAchievements class, and all it is is a glorified dictionary, right?

I mean, completeAchievement: adds some convenience, but if we want to reach in and check percentage complete for example, it's really inconvenient.

So, step one, let's make a singleton.

I usually recommend against singletons, but in this case, it never makes sense to instantiate more than one object of this class, though it does make sense that multiple objects might want to work with this object, and the data needs to be consistent throughout the app.

This is what the singleton pattern looks like in Objective-C:

+ (DTAchievements *)achievements {
    static DTAchievements *achievements;
    @synchronized (self) {
        if (!achievements) {
            achievements = [[self alloc] init];
        }
        return achievements;
    }
}

Now let's add a property for each achievement. This will make the class much easier to work with than a dictionary full of achievements.

@property (nonatomic,strong) DTAchievement *hundredFloorsRevealed;
// etc.

Of course, if we do this, we will either have to instantiate them all in init, or we can use lazy instantiation:

- (DTAchievement *)getHundredFloorsRevealed {
    if (!_hundredFloorsRevealed) {
        _hundredFloorsRevealed = [[DTAchievement alloc]initWithType:AchievementTypeTopFloorRevealed];
    }
    return _hundredFloorsRevealed;
}

This also eliminates the need for the enum, and that big switch statement.

It's weird to me that we can, through the DTAchievements, set an achievement to complete status, however there's seemingly no way of effecting an achievement's percentage complete status.

I also think that CGFloat isn't a useful return type for percentage complete. We're most likely calculating this for display to the user, right? Does a user really care that he's 66.66666666666666667% complete? Is displaying 67% fine enough?

Even if you want to use a floating point number, I'm very curious as to why you chose CGFloat here rather than float or double when you've consistently used int rather than NSInteger throughout all the rest of your code. If you were going to avoid the Objective-C typedef with one or the other, I think you've picked the wrong one.

StackExchange Code Review Q#59847, answer score: 3