Is Apple's singleton sample code correct?
Is Apple's singleton sample code correct?
- Subject: Is Apple's singleton sample code correct?
- From: David Gimeno Gost <email@hidden>
- Date: Fri, 25 Nov 2005 18:14:06 +0100
Hello!
This is my first post to the list. I've been reading it for a few
months.
The sample code shown here doesn't feel right to me:
http://developer.apple.com/documentation/Cocoa/Conceptual/CocoaObjects/
Articles/CreateSingleton.html
1. I believe there is a bug in the following class method:
+ (id)allocWithZone:(NSZone *)zone
{
@synchronized(self) {
if (sharedGizmoManager == nil) {
return [super allocWithZone:zone]; // -> Is this correct?
}
}
return sharedGizmoManager;
}
The above method is supposed to "ensure that only the singleton is
returned if someone tries to allocate and initialize an instance of
your class directly". Well, it seems to me that this only works if
sharedGizmoManager != nil. Shouldn't it be
sharedGizmoManager = [super allocWithZone:zone];
instead of
return [super allocWithZone:zone];
?
As it stands now, unless the class creation method is used, each
invocation of [[MyGizmoClass alloc] init] will create a new instance
(sharedGizmoManager will always be nil, no matter how many "singleton"
instances have been created).
2. Regardless of whether this is indeed a bug or not, calling
[[MyGizmoClass alloc] init] more than once is still unsafe, unless
special care is taken to ensure that MyGizmoClass' -init method can
safely be called repeatedly on the same object. Thus, even if the code
above was correct, the problem it tries to resolve still remains,
albeit in a different subtler form.
3. Assuming I'm right on the first one, isn't the assignment to
sharedGizmoManager in the class creation method redundant? I think
there should be only one method where the shared instance variable is
set:
+ (MyGizmoClass*)sharedManager
{
@synchronized(self) {
if (sharedGizmoManager == nil) {
// Assignment is redundant, already done in +allocWithZone:
/*sharedGizmoManager =*/ [[self alloc] init];
}
}
return sharedGizmoManager;
}
4. What's the point in overriding the -retain, -release, and
-autorelease methods? As written in the example, these methods will
silently allow the programmer to write buggy code that doesn't follow
Cocoa's memory management rules. Is this intended behavior? Please,
don't tell me they are worth writing for efficiency reasons. A
singleton is not the kind of object I expect to put in a container very
often and I would consider explicitly retaining/releasing it a bug in
most cases. I bet the code size overhead caused by overriding these
methods outweighs any other hypothetical speed gains by several orders
of magnitude -- assuming they could be measured, that is :-)
5. Does allowing calling -copy and -copyWithZone: on singletons make
sense? If this class had to be written in C++, the copy constructor and
assignment operator would be declared private and left undefined to
ensure that the compiler forbids any attempt to copy the singleton
object. Unfortunately, there is no way to do that in Objective-C, but
shouldn't we at least try to catch such errors at run time?
Similar considerations could be made about the convenience of allowing
users of the singleton to bypass the class creation method and create
the shared instance directly by calling [[alloc] init].
To summarize, possible bugs and/or omissions aside, it seems to me that
Apple's singleton sample code strives to silently allow users of the
singleton class to "safely" misuse it. Is this considered good design
in Objective-C? It is contrary to well established practices in
statically-typed languages such as C++ where introducing constructs to
detect misuses ASAP (i.e. at compile-time) is considered good design.
I believe that not being able to detect some coding errors at compile
time in Objective-C is bad enough and that we should at least try to
detect them at run time. Is it considered good design to let them go
undetected and program defensively against them instead? I think that's
the wrong path and a sure way to introduce new bugs as #1 demonstrates.
Finally, for those brave enough to have read so far :-), there is one
more thing related to #2 that has bugged me since I first read "The
Objective-C Programming Language". The designated initializer of a
class is supposed to be written like this:
- (id) init
{
if ( self = [super init] ) {
// ...
}
return self;
}
The assignment inside the if clause is supposed to take care of the
possibility that the superclass' initialization method returns a
different object or nil if the initialization failed. But it doesn't.
It just takes care of the case when the superclass initialization fails
and returns nil. What happens if the superclass -init method succeeds
and does indeed return a different (not nil) object? The subclass
initialization method will proceed as if nothing strange had happened,
happily creating new instance variables and ignoring existing ones.
Yes, it will "correctly" return the same object as the one returned by
the superclass, but is this indeed correct? Shouldn't it return that
object untouched if it's not the same as the one that received the
message? If it is indeed appropriate to (re)initialize that object,
shouldn't any existing instance variables be released to avoid memory
leaks and ensure proper deallocation of resources.
I wonder how many people take these considerations into account when
writing -init methods. I've certainly seen no mention of these issues
anywhere in the Obective-C literature I've read so far. Admittedly, it
sure doesn't matter in most cases... until one tries to write a class
that does some clever things, such as overriding the +allocWithZone
class method to allow [[alloc] init] be called on a singleton.
Regards.
_______________________________________________
Do not post admin requests to the list. They will be ignored.
Cocoa-dev mailing list (email@hidden)
Help/Unsubscribe/Update your Subscription:
This email sent to email@hidden