I just started coding in Objective-C and would like to know if my simple implementation of a Stack is acceptable & what ways would you improve the Stack code, or the Main code? i'm curious of things such as (but not limited to):
- formatting
- edge cases
- run time (like one part where try to manipulate strings in
description
) - general good practice
- memory management practices (i'm using ARC in this code)
Stack.h:
#import <Foundation/Foundation.h>
@interface Stack : NSObject
-(void)push:(id)obj;
-(id)pop;
-(NSUInteger)size;
-(id)peek;
-(BOOL)isEmpty;
@end
Stack.m file:
#import "Stack.h"
@implementation Stack{
NSMutableArray *stack;
}
-(id)init{
self = [super init];
if(self!=nil){
stack = [[NSMutableArray alloc] init];
}
return self;
}
-(void)push:(id)obj{
[stack addObject:obj];
}
-(id)pop{
id lastObj = [stack lastObject];
[stack removeLastObject];
return lastObj;
}
-(NSUInteger)size{
return stack.count;
}
-(id)peek{
return [[stack lastObject] copy];
}
-(BOOL)isEmpty{
return stack.count == 0;
}
-(NSString *)description{
NSMutableString *result = [[NSMutableString alloc] initWithString:@"["];
for (id s in stack) {
[result appendFormat:@"%@, ",[s description]];
}
if (stack.count>0) {
result = [[result stringByTrimmingCharactersInSet:[NSCharacterSet characterSetWithCharactersInString:@", "]] mutableCopy];
}
[result appendString:@"]"];
return result;
}
@end
Lastly, this is how i verify my code is "working":
#import <Foundation/Foundation.h>
#import "Stack.h"
int main(int argc, const char * argv[])
{
@autoreleasepool {
Stack *s1 = [[Stack alloc] init];
assert([s1 size]==0);
assert([s1 pop]==nil);
assert([s1 peek]==nil);
assert([s1 isEmpty]==YES);
NSLog(@"first set of tests passed");
[s1 push:[NSNumber numberWithInt:0]];
[s1 push:[NSNumber numberWithInt:3]];
[s1 push:[NSNumber numberWithInt:5]];
NSLog(@"%@",[s1 description]);
assert(![s1 isEmpty]);
assert(s1.size == 3);
assert([[s1 peek] isEqual:[NSNumber numberWithInt:5]]);
assert([[s1 pop] isEqual:[NSNumber numberWithInt:5]]);
assert(s1.size == 2);
assert([[s1 pop] isEqual:[NSNumber numberWithInt:3]]);
assert(![[s1 pop] isEqual:[NSNumber numberWithInt:3]]);
assert([s1 pop] == nil);
assert(s1.isEmpty);
NSLog(@"second set of tests passed");
}
return 0;
}
1 Answer 1
-(id)pop{
id lastObj = [stack lastObject];
[stack removeLastObject];
return lastObj;
}
Not sure if you are using ARC, but without ARC this is a potential use-after-free bug. If you are not using ARC you should consider:
-(id)pop{
id lastObj = [stack lastObject];
if (lastObj) // < -- now checking for nil
{
[[lastObj retain] autorelease]; // < -- this line added
[stack removeLastObject];
}
return lastObj;
}
(This also assumes that the objects in the stack are NSObject
s, but I believe NSMutableArray
makes the same assumption...)
This bumps the ref count up so that the next line (which will cause the NSMutableArray
to release its reference to the object) does not end up deallocating the object.
Update on second reading:
Also, what do you do when lastObj
is nil
(i.e. too many pop
s)? Doesn't seem like you've handled that case.
-
\$\begingroup\$ sorry, forgot to mention i'm using ARC, the edit is in now. Oh, and when you do
[[lastObj retain] autorelease];
in non-ARC code, do you have to also manually release thelastObj
later? like... in thedealloc
? or where? \$\endgroup\$David T.– David T.2012年11月17日 01:23:00 +00:00Commented Nov 17, 2012 at 1:23 -
\$\begingroup\$ @DavidT. -
retain
increments the count, thenautorelease
will set it up to be decremented later. (When the pool is drained, usually inside the run loop.) When the count reaches 0 (insiderelease
) thendealloc
is called. \$\endgroup\$asveikau– asveikau2012年11月17日 01:32:01 +00:00Commented Nov 17, 2012 at 1:32