I need code review on using singleton to access local database

Question

As I get to some performance issues in my app and find that I use database access in a bad way.
So I decided to move to singleton pattern.
I need someone to review this code and confirm me that I made a good database access via singleton pattern or I am doing something wrong:
This is the class:

@interface DataAccessController : NSObject{
 sqlite3 *databaseHandle;
}
+ (id)sharedManager;
-(void)initDatabase;
...
+ (id)sharedManager {
 static DataAccessController *sharedMyManager = nil;
 static dispatch_once_t onceToken;
 dispatch_once(&onceToken, ^{
 sharedMyManager = [[self alloc] init];
 });
 return sharedMyManager;
}
- (id)init {
 if (self = [super init]) {
 [self initDatabase]; // open database connection
 }
 return self;
}
...

AppDelegate.m

DataAccessController *d = [DataAccessController sharedManager];

Usage through the application every time I need data I use:

DataAccessController *d = [DataAccessController sharedManager];
NSMutableArray* data = [d getAllRecordedUnits];

• Flambino: To my untrained eye this looks like a straight-forward, by-the-book singleton construction. If there are still performance bottlenecks in your code, they're probably elsewhere
• Gordon Childs: There's no need to initialize sharedMyManager to nil.
• Max: There's a good chance that the bottleneck is not with your application but with your database. You mentioned it's a local database server so network shouldn't be a problem. How do you use your singleton to access the database and what do you access? Any performance heavy querys perhaps?
• JRG-Developer: @RhythmicFistman, opinions differ on this. In this instance, there isn't a need to set sharedMyManager to nil explicitly, but I would as a general practice when creating variables.
• Gordon Childs: Sure, it's a bug waiting to happen if you decide to replace dispatch_once with a simple if statement. I think that since dispatch_once became the singleton idiom it is often misused in two ways: 1. unnecessarily, in singletons that are intended to be single threaded only and 2. as magic DWIM dust, in multithreaded singletons in which dispatch_once is the author's first and last attempt at synchronization.

Answer 1

Based on your provided code, this is the correct way to create a singleton object.

In such, here's a reference link from Matt Gallow's site showing the same singleton setup as you're doing.

However, there is not enough information provided to determine the cause of your performance issues (whether or not caused by accessing the database). In general, writing efficient database code is hard, which is why Core Data exists.

You might consider looking into Core Data to see if this will work better for your app or perhaps creating a question on StackOverflow describing the issue you're seeing in more detail and asking for help on how to implement what you're trying to do.

Answer 2

It's the same pattern I always use for "singletons".

I have two more picky comments.

1. Move the declaration of the databaseHandle instance variable into the implementation. With modern Objective-C, you should never have instance variables in your interface declaration.
2. Rename -initDatabase. The convention is that methods beginning with init... are initialisers. Perhaps call it openDatabase.