1

Objective-c noob here. I'm experiencing a EXC_BAD_ACCESS error in my tests and can't figure out why. I've written a Cart class for my application representing a user's cart. A Cart can have multiple Orders, stored in an array. I'm attempting to implement KVO compliance for the Orders array with the given code (Apologies for the long code snippet, want to make sure everything is present to resolve the problem). The .h file:

#import "Model.h"

@class Item;
@class Organization;
@class Order;

@interface Cart : NSObject

@property (copy, readonly, nonatomic) NSArray *orders;

- (instancetype) init __attribute__((unavailable("init not available. Use `sharedCart`.")));
+ (instancetype)sharedCart;

- (void)setItem:(Item *)item withQuantity:(NSNumber *)quantity;
- (void)removeItem:(Item *)item;
- (NSNumber *)itemCount;
- (Order *)orderForSeller:(Organization *)seller;

@end

And the .m:

@interface Cart()
- (void)addOrder:(Order *)order;
- (void)removeOrder:(Order *)order;
@property (copy, readwrite, nonatomic) NSArray *orders;
@end

NSString * const ordersKey = @"orders";

@implementation Cart
{
    NSMutableArray *_orders;
}

@synthesize orders = _orders;


+ (instancetype)sharedCart {
    static dispatch_once_t onceToken;
    static Cart *cart;

    dispatch_once(&onceToken, ^{
        cart = [[Cart alloc] initPrivate];
    }); 

    return cart;
}

- (void)insertObject:(Order *)order inOrdersAtIndex:(NSUInteger)index {
    [_orders insertObject:order atIndex:index];
}

- (void)removeObjectFromOrdersAtIndex:(NSUInteger)index {
    [_orders removeObjectAtIndex:index];
}

- (void)setOrders:(NSArray *)array {
    if (array != _orders) {
        _orders = [array mutableCopy];
    }   
}

- (instancetype)initPrivate {
    self = [super init];
    if (self) {
        self = [super init];
        _orders = [[NSMutableArray alloc] init];
    }   
    return self;
}
+ (instancetype)object {
    @throw [NSException exceptionWithName:NSInternalInconsistencyException reason:@"Can not create new Cart instance. Used [Cart sharedCart]" userInfo:nil];
}

-(Order *)orderForSeller:(Organization *)seller {
    for (Order *order in self.orders) {
        if ([order.seller is:seller]) {
            return order;
        }
    }
    return nil;
}

- (void)setItem:(Item *)item withQuantity:(NSNumber *)quantity {

    Order *order = [self orderForSeller:item.seller];

    if (!order) {
        order = [Order object];
        order.seller = item.seller;
        order.user = [User currentUser];
        [self addOrder:order];
    }

    [order setItem:item withQuantity:quantity];
}
-(void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary *)change context:(void *)context {


    // If an order is ever empty, remove it from cart.
    if ([object isKindOfClass:[Order class]] && [keyPath isEqualToString:@"orderItems"]) {
        Order *order = (Order *)object;
        if ([[order itemCount] integerValue] == 0) {
            [self removeOrder:order];
        }
    }

    // When orders are saved, remove them from cart.
    if ([object isKindOfClass:[Order class]] && [keyPath isEqualToString:@"isNew"]) {
        Order *order = (Order *)object;
        BOOL orderHasBeenPlaced = ![order isNew];
        if (orderHasBeenPlaced) {
            [self removeOrder:order];
        }
    }
}
- (void)addOrder:(Order *)order {
    [order addObserver:self forKeyPath:@"orderItems" options:NSKeyValueObservingOptionNew context:nil];
    [order addObserver:self forKeyPath:@"isNew" options:NSKeyValueObservingOptionNew|NSKeyValueObservingOptionOld context:nil];
    [self insertObject:order inOrdersAtIndex:[_orders count]];
}

-(void)removeOrder:(Order *)order {
    [order removeObserver:self forKeyPath:@"orderItems"];
    [order removeObserver:self forKeyPath:@"isNew"];
    NSUInteger index = [_orders indexOfObject:order];
    [self removeObjectFromOrdersAtIndex:index];
}

-(void)removeItem:(Item *)item {
    Order *order = [self orderForSeller:item.seller];
    if (order) {
        [order removeItem:item];
    }
}

-(NSNumber *)itemCount {
    return [self.orders valueForKeyPath:@"@sum.itemCount"];
}

- (void)dealloc {
    for (Order *order in self.orders) {
        [order removeObserver:self forKeyPath:@"isNew"];
        [order removeObserver:self forKeyPath:@"orderItems"];
    }
}

@end

The addOrder: method is producing the EXC_BAD_ACCESS exception on this line:

[self insertObject:order inOrdersAtIndex:[_orders count]];

Debugging, the _orders array is a valid NSMutableArray. Anyone have any idea what's going on? Much appreciated.

`

Bibs
  • 995
  • 1
  • 8
  • 17
  • Run your code with NSZombieEnable diagnostic flag that'll help you find why that bad access is happening. https://stackoverflow.com/questions/5386160/how-to-enable-nszombie-in-xcode – Nimrod Gutman May 02 '15 at 16:44

1 Answers1

2

I'm pretty sure I suggested this on one of your other questions: the problem is presumably with something that is key-value observing the orders property of an instance of this Cart class. Probably an observer did not stop observing before it was deallocated. So, when you modify your orders property, KVO tries to send a change notification to a no-longer-existing object, which crashes.

Nimrod's suggestion to run your app under the Zombies instrument is a good one.

By the way, for this Cart class:

  • You should always specify a unique context value when calling -addObserver:forKeyPath:options:context: and check for it in your -observeValueForKeyPath:ofObject:change:context: method. For contexts other than the one you're using, call through to super and return. The frameworks are permitted to make your objects observe stuff, too, and if you fail to do this you will interfere with their functioning.

  • You should move the -addObserver:... and -removeObserver:... calls from -addOrder: and -removeOrder: to the property mutation methods, -insertObject:inOrdersAtIndex:, -removeObjectFromOrdersAtIndex:, and -setOrders:. Basically, you should move the code to the same place where you add and remove items in the actual array, so you're sure that you're always observing all objects in the array and you stop observing objects removed from the array. In particular, you neglected the -setOrders: case, although I suspect you never use that method.

    In any case, having the array manipulation in a separate place from the observation manipulation leaves you open to doing one without the other. For example, you might add future calls to -insertObject:inOrdersAtIndex: that don't go through -addOrder: and you'd fail to start observing that order.

Ken Thomases
  • 88,520
  • 7
  • 116
  • 154