On 19 Jun 2014, at 10:57 am, Trygve Inda <[email protected]> wrote:
> The method propertyKeys (below) is used to simplify and shorten the code in
> these classes since I want to encode/decode and (upon dealloc), release all
> the properties.
> -(void)dealloc
> {
> for (NSString* key in [self propertyKeys])
> [[self valueForKey:key] release];
This looks pretty smelly.
I can understand your motivation, but I think this could end up being
problematic if later you want to generalise or subclass your object. If a
subclass adds to the -propertyKeys, it could find them over-released, because
the base class has already released them (or else it will need to rely on this
implementation detail of the base class, which is knowledge it shouldn't
require). If it overrides any property getter to do something different,
releasing the result is probably going to be wrong. If you declare any
properties anywhere to be anything other than 'retain' it's going to be wrong.
You might argue that you don't intend to subclass or generalise or use any
properties other than 'retain', and it works as it stands, and that would be
true, but there is a more generic way that is no more work.
Ideally, your -dealloc method should mirror your -init method, so that you have
each property set to nil listed individually, just as -init sets each one
individually. Sure, that's tedious if you have hundreds of properties, but it's
at least reliable with no nasty surprises. If you are determined to iterate a
list, then at least use the setter, passing nil, rather than the getter, and
calling release. (That also allows you to change a property to 'assign' or
'copy' if necessary without creating a bug).
I ought to mention that I used something very close to your scheme a while ago
to try and save myself a load of tedious coding to init, archive, dearchive and
dealloc a whole bunch of properties. At first it seemed like a huge win, but
I've been regretting and undoing it ever since, because every time I come to
actually use one of these objects, I find these shortcuts not doing exactly
what they need to do. Having said that I am subclassing, often in ways I didn't
originally expect to be doing.
> -(NSArray *)propertyKeys
> {
> NSMutableArray* propertyKeyArray =
> [[[NSMutableArray alloc] init] autorelease];
Just as a final comment, why not make this a class method, and cache the array
in a static? It never changes for a given class, so rebuilding it every time is
an unnecessary waste of time. OK, that is possibly premature optimisation, but
this sort of case is something that makes sense to "optimise" by design whether
it matters or not.
It would probably also be better if this method didn't automatically enumerate
the properties using property_getName, but built it from explicit string
literals (even though you then have to list them individually). It means
properties added by a subclass are exclusively the domain of the subclass, as
they should be, and that properties that you want to exclude from archiving,
KVO, etc can be simply omitted from the list as needed.
So my arguments against your approach boil down to problems if you subclass,
which you will probably say you never intend to do. I thought that I wouldn't
be subclassing as well :)
A performance related argument: using property getters and setters in
-initWithCoder: and -encodeWithCoder: can bring with them serious performance
issues. Might not matter in your case, or in most cases, but it really adds up
if you have a large and complex object graph. A recent exercise to set ivars
directly in -initWithCoder: instead of using property setters saw a 100 - 600x
speed improvement when reading a file for a graph with ~10,000 objects. Just
sayin'.
--Graham
_______________________________________________
Cocoa-dev mailing list ([email protected])
Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com
Help/Unsubscribe/Update your Subscription:
https://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com
This email sent to [email protected]
