Re: Is Apple's singleton sample code correct?
Re: Is Apple's singleton sample code correct?
- Subject: Re: Is Apple's singleton sample code correct?
- From: Shawn Erickson <email@hidden>
- Date: Fri, 25 Nov 2005 10:05:27 -0800
On Nov 25, 2005, at 9:14 AM, David Gimeno Gost wrote:
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];
?
Yes. File a documentation bug about it.
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.
Consider reviewing what I use...
<http://www.cocoadev.com/index.pl?FTSWAbstractSingleton>
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;
}
It depends on how normal you want to make your object behave (recall
it is a subclass of NSObject so it has several class and instance
methods that code may expect to be able to use).
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?
It follows the memory management rules, well may be said that is
allows callers to follow the memory management rules without caring
that the object is really a singleton. It just happens in the
implementation of the methods used that it results in nothing being
done.
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 :-)
What if you have to pass a reference to the singleton around in code
that assumes it must retain it for it to continue to exist. You want
to special case all such pathways to support singletons?
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?
Note that immutable classes like NSString implement copy with a
simple retain since it doesn't make sense to make a copy of something
that wont change. Same thing applies to singletons.
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.
Up to how you want to handle it but consider that being able to treat
a singleton as just any other object can simplify coding since from
the outside nothing looks / acts different about it. It also would
allow you to switch it from being a singleton in the future, assuming
folks just treated it like any other object.
In general Cocoa paradigm is about not trying to overly special case
things but to provide consistency when it is possible. This allows
one to follow a few set of rules which is easy to program under and
to maintain going forward.
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.
Yes super could return a class different then what you happened to be
before the call. It is required that the object return by [super
init] has to implement the same set of methods that your super class
implements and ideally have the same set of public fields available.
In general it is expected that you use messages to accessors to
initialize your object, this will protect you from the issue you
outline.
-Shawn
_______________________________________________
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