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: David Gimeno Gost <email@hidden>
- Date: Sat, 26 Nov 2005 04:50:37 +0100
On 25 Nov 2005, at 19:05, Shawn Erickson wrote:
Yes. File a documentation bug about it.
I will, but I want to clarify things first. BTW, what's the recommended
procedure to report such bugs, the feedback form available at the
bottom of the html pages or bugreport.apple.com? (I have the feeling
this has already been discussed in one of these mailing lists.)
Consider reviewing what I use...
<http://www.cocoadev.com/index.pl?FTSWAbstractSingleton>
I believe I don't fully understand it because it seems to me that your
implementation has the same problems I've found in Apple's sample code
(except the bug in +allocWithZone:) and then adds a few more.
Singletons based on your code allow the shared instance to be obtained
by means of an [[alloc] init] message, but they must be written as
[[alloc] initSingleton] instead. Why? I thought the point of allowing
the shared instance to be returned by [[alloc] init] was to allow the
singleton class be used as any other class without having to take into
consideration the fact that we are dealing with a singleton. While I'm
not sure this is a good idea, it does make sense, but the way you've
implemented it doesn't, at least to me.
First, if I try to get a singleton instance by means of [[alloc]
initSingleton] I'm explicitly stating that I'm dealing with a
singleton. Why would someone in his/her right mind prefer [[alloc]
initSingleton] to +sharedInstance if he/she already knows he/she is
dealing with a singleton?
Second, your code calls the -initSingleton method in the method called
by +allocWithZone:. Not only doesn't this avoid the problem of possible
multiple -init invocations on the same object, it guarantees that the
problem will happen even if [[alloc] initSingleton] is called only
once.
I don't get it, sorry.
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).
I don't know what you're talking about here, sorry. What I'm saying is
that if one writes the +allocWithZone: as suggested in Apple's sample
code and fixes the bug by writing
sharedInstance = [super allocWithZone:zone];
instead of
return [super allocWithZone:zone];
in the +allowWithZone: method, then the assignment to the
sharedInstance static variable in the +sharedInstance method becomes
redundant and can be commented out.
I don't see how what you've replied affects in any way to what I'm
saying. Anyway this is unimportant, I just pointed it out because doing
that would have allowed the bug to be detected from the very beginning.
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.
But you don't need to override the methods to accomplish that. What I'm
saying is that the default (inherited) methods already do the right job
if users of the class follow the Cocoa's memory management rules. The
only thing you accomplish overriding those methods in that way is the
impossibility to detect bugs in client code that doesn't follow the
rules.
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?
What's wrong with letting the default (inherited) methods do their job?
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.
No, it's not the same thing. I'm talking about semantics here, not
about implementation details. That immutable objects implement copy
with a simple retain is an implementation detail, it doesn't change the
semantics of the copy operation in any way. That it doesn't make sense
to copy a singleton object is a semantic question completely unrelated
to the way -copy is actually implemented for any particular class. I
would consider trying to copy a singleton object a bug in most (all?)
cases and I'd rather try to detect such possible bugs than silently
ignore them, at least during development and testing.
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.
OK, I can buy that. I'm not sure it's a good idea in all cases but it
does make sense. I still believe the way the current Apple's sample
code implements the idea is wrong and/or incomplete, however:
1. If I wanted to allow getting the singleton instance variable through
[[alloc] init] calls, then I would write +allocWithZone: as follows:
+ (id) allocWithZone: (NSZone*) zone
{
@synchronized( self ) {
if ( sharedInstance == nil ) {
sharedInstance = [super allocWithZone: zone];
} else {
[sharedInstance retain];
}
}
return sharedInstance;
}
This will work correctly regardless of whether the -retain and -release
methods are overridden or not. Moreover, it encourages client code to
send a -release message to any singleton instance it gets through
[[alloc] init], as it is supposed to do with any object returned by
such methods, because failure to doing so can be detected with the aid
of debugging tools, as for any other resource leak.
The writer of the singleton class still has to be aware that the -init
method can be repeatedly called for the same (already initialized)
object.
2. The -retain and -release methods shouldn't be overridden. There is
no point in doing so and the way it is done in the current sample code
guarantees that code breaking Cocoa's memory management rules will go
by unnoticed.
3. The -copyWithZone: method should be overridden to, at the very
least, log any attempts to clone a singleton object during
development/testing. For production code it should just return self if
that's the intended behavior or raise an exception if sending a -copy
message to the singleton is considered a bug in all cases.
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