Re: Memory Management: Revamped Engine Example
Re: Memory Management: Revamped Engine Example
- Subject: Re: Memory Management: Revamped Engine Example
- From: Shawn Erickson <email@hidden>
- Date: Sun, 01 Jan 2006 08:33:14 -0800
On Jan 1, 2006, at 2:12 AM, Jordan Evans wrote:
1. I added a EnginePart Class and made each engine
part a subclass of the EnginePart Class.
2. I removed the superfluous pointer.
3. I added the NSMutableIndexSet.
Thanks Chris Hanson.
I have to get some more practice in, but I think the
NSIndexSet is going to be handy. Is there anything
else that could be done to straighten this thing up?
What is this code going to be used for? I sure hope it isn't being
used for class work since you had the list basically do your work :(
Source code attached.
#import <Foundation/Foundation.h>
//EnginePart Class
@interface EnginePart : NSObject {} @end
@implementation EnginePart @end
The above class represents a single engine part of a particular type
so drop the plural.
...snip...
- (id)init; <---";" here is bad
{
int i;
self = [super init];
if (self != nil) {
enginePartsArray = [[NSMutableArray alloc] init];
[enginePartsArray addObject:[[[CombustionChamber alloc] init]
autorelease]];
[enginePartsArray addObject:[[[Compressor alloc] init] autorelease]];
[enginePartsArray addObject:[[[FuelInjector alloc] init]
autorelease]];
[enginePartsArray addObject:[[[FuelInjector alloc] init]
autorelease]];
[enginePartsArray addObject:[[[Shaft alloc] init] autorelease]];
[enginePartsArray addObject:[[[Turbine alloc] init] autorelease]];
}
return self;
}
You could also use autorelease as shown above.
- (void)dealloc; <---";" here is bad
{
- (void)maintenance:(NSMutableIndexSet*)badParts
{
int i, part, partCount;
id PartClass;
NSLog(@"Parts before maintenance:\n");
for(i=0; i<[enginePartsArray count]; i++)
NSLog(@"Part%i: %@", i, [[enginePartsArray
objectAtIndex:i] description]);
partCount = [badParts count];
Likely clearer to use "badPartCount".
for( i=0; i<partCount; i++ )
{
part = [badParts firstIndex];
Likely clearer to use "partIndex" in the above.
PartClass = [[enginePartsArray objectAtIndex:part] class];
Upper case variables is not normal in Cocoa so "partClass".
[enginePartsArray replaceObjectAtIndex:[badParts firstIndex]
withObject:[[PartClass alloc] init]];
[[enginePartsArray objectAtIndex:part] release];
You could write the above two lines as ...
[enginePartsArray replaceObjectAtIndex:part withObject:[[[PartClass
alloc] init] autorelease]];
[badParts removeIndex:part];
}
What is the point of the "maintenance" method? I guess the intent is
to replace a part with a new one... in other words parts will degrade
over time/use and you just haven't got that far in your
implementation? If that is the case then "badParts" and "maintenance"
isn't likely the best name. How about replaceParts:(NSMutalbeIndex*)
badParts.
Note in the above method you are not doing anything to validate that
an index in badParts is a valid index for a engine part (falls inside
your enginePartsArray). Likely that is ok since it should be an
exceptional case to have that happen and so handling it by an
exception (objectAtIndex: will throw one if index is out of bounds)
however keep that in mind when implementing you loop.
Finally since you are removing stuff from the index set as you go the
value "partCount" become disconnected from the current start of the
set. As your loop is currently written that side effect isn't an
issue but it could come to bite you in the future if you modify the
code and don't recall that side effect. So either document it with a
comment in the code or rework you loop. I suggest you drop the use of
partCount all together and do something like the following...
while ((part = [badParts firstIndex]) != NSNotFound) {
....
}
Also instead of modifying the index set (which the caller of
maintenance may not expect) consider...
part = [badParts firstIndex];
while (part != NSNotFound) {
....
part = [badParts indexGreaterThanIndex:part];
}
...or you could invert the operation of your loop and iterate over
engine parts checking if their index was in the index set...
...or you could make a mutableCopy of the index set before your loop...
-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