Re: Memory Management: Revamped Engine Example
Re: Memory Management: Revamped Engine Example
- Subject: Re: Memory Management: Revamped Engine Example
- From: Chris Hanson <email@hidden>
- Date: Sat, 31 Dec 2005 20:29:01 -0800
On Dec 31, 2005, at 6:31 PM, Jordan Evans wrote:
I think this engine is getting close to being a fine
runner. But I'm not sure if it's tip-top yet.
I think you're still a bit of a ways from understanding how Cocoa
memory management works. You're going through a lot of work here for
not a whole lot of benefit.
I think you should take another look at the Cocoa memory management
documentation. It really is pretty straightforward, especially
compared to what you're doing below.
#import <Foundation/Foundation.h>
//Filler Classes for Engine Class
@interface CombustionChamber : NSObject{} @end
@implementation CombustionChamber @end
@interface Compressor : NSObject{} @end
@implementation Compressor @end
@interface FuelInjector : NSObject{} @end
@implementation FuelInjector @end
@interface Shaft : NSObject{} @end
@implementation Shaft @end
@interface Turbine : NSObject{} @end
@implementation Turbine @end
If this were real code, I would suggest making these a subclass of a
base EnginePart class.
@interface Engine : NSObject
{
NSMutableArray *engineParts;
id *enginePartsPtr;
This is superfluous. You don't need it, you have the engine parts in
an array already.
}
- (id)init;
- (void)maintenance:(NSArray*)badParts;
@end
@implementation Engine
- (id)init;
{
[super init];
engineParts = [[NSMutableArray alloc] init];
//Here are the parts for this engine. I have added
two Fuel Injectors.
All of these should be released after being added to the engineParts
collection. Collections retain objects that are added to them.
Furthermore, you're not keeping individual references to them around,
so you should give up ownership of them.
[engineParts addObject:[[CombustionChamber alloc]
init]];
[engineParts addObject:[[Compressor alloc] init]];
[engineParts addObject:[[FuelInjector alloc] init]];
[engineParts addObject:[[FuelInjector alloc] init]];
[engineParts addObject:[[Shaft alloc] init]];
[engineParts addObject:[[Turbine alloc] init]];
Removed superfluous references to enginePartsPtr and i.
return self;
}
- (void)dealloc;
{
[engineParts release];
Note that you missed invoking free() on enginePartsPtr here. Which
will be just fine if you remove it.
[super dealloc];
}
- (void)maintenance:(NSArray*)badParts
{
Since badParts isn't a collection of parts but part indexes, I'd name
it badPartIndexes. (I would also take a look at NSIndexSet for a use
like this.)
int i, part;
id PartClass;
NSLog(@"Parts before maintenance:\n");
for(i=0; i<[engineParts count]; i++)
NSLog(@"Part%i: %@", i, [[engineParts
objectAtIndex:i] description]);
for( i=0; i<[badParts count]; i++ )
{
part = [[badParts objectAtIndex:i] intValue];
PartClass = [[engineParts objectAtIndex:part]
class];
[enginePartsPtr[part] release];
Don't do this. If you apply my changes above, what you're doing here
is releasing an object out from under a collection. As far as the
collection knows, it still has a retain on that object and so before
removing it, the collection will send it a release. But it might
*already* be released, which means that the release message could be
going anywhere.
[engineParts replaceObjectAtIndex:[[badParts
objectAtIndex:i] intValue] withObject:[[PartClass
alloc] init]];
This creates a new part and replaces an object in the collection with
it. The object in the collection is sent a -release and your new
part is sent a -retain. Since you're giving up ownership of the
created part to the collection, you should -release the new part.
Also removed superfluous references to enginePartsPtr.
}
NSLog(@"Parts after maintenance:\n");
for(i=0; i<[engineParts count]; i++)
NSLog(@"Part%i: %@", i, [[engineParts
objectAtIndex:i] description]);
printf("\n");
}
@end
int main (int argc, const char *argv[])
{
NSAutoreleasePool *pool = [[NSAutoreleasePool alloc]
init];
Engine *myEngine = [[[Engine alloc] init]
autorelease];
NSArray *badParts1 = [NSArray arrayWithObjects:
[NSNumber numberWithInt:0], [NSNumber
numberWithInt:1], nil ];
NSArray *badParts2 = [NSArray arrayWithObjects:
[NSNumber numberWithInt:5], nil ];
[myEngine maintenance:badParts1];
[myEngine maintenance:badParts2];
[pool release];
return 0;
}
This should be fine.
-- Chris
_______________________________________________
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