2
\$\begingroup\$

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):

  1. formatting
  2. edge cases
  3. run time (like one part where try to manipulate strings in description)
  4. general good practice
  5. 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;
}
\$\endgroup\$
1
\$\begingroup\$
-(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 NSObjects, 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 pops)? Doesn't seem like you've handled that case.

\$\endgroup\$
  • \$\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 the lastObj later? like... in the dealloc? or where? \$\endgroup\$ – David T. Nov 17 '12 at 1:23
  • \$\begingroup\$ @DavidT. - retain increments the count, then autorelease will set it up to be decremented later. (When the pool is drained, usually inside the run loop.) When the count reaches 0 (inside release) then dealloc is called. \$\endgroup\$ – asveikau Nov 17 '12 at 1:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.