Trying to implement Key-Value Observing for first time, get some error

Question

I have trouble implementing a Key-Value Observer at my attempt to follow the MVC pattern. I have a controller class, a model class and a view class. I update my model from the controller class and I want to put a key value observer in my view class to monitor when a NSMutableArray changes in model (like through addObject) and then redraw itself automatically. I used answer in this thread to guide me: How to add observer on NSMutableArray?

Code so far:

From my Scene (using sprite kit if it matters). Setting of letters will be done from Ctrl class, this is just to test.

BarCtrl *barCtrl = [[BarCtrl alloc] init];
BarModel *barModel = [[BarModel alloc] init];
BarView *barView = [[BarView alloc] init];

barCtrl.barModel = barModel;
barCtrl.barView = barView;
barView.barModel = barModel;

ScrabbleDeck *sd = [[ScrabbleDeck alloc] init];

if([barModel addLetter:[sd getLetter] onSide:BarModelSideRight])
    NSLog(@"Added letter");

BarModel.h

#import <Foundation/Foundation.h>
#import "Letter.h"

typedef NS_ENUM(int, BarModelSide) {
    BarModelSideLeft,
    BarModelSideRight
};

@interface BarModel : NSObject

@property (nonatomic, strong) NSMutableArray *addedLetters;

- (instancetype)init;
- (BOOL) addLetter: (Letter*) letter onSide: (BarModelSide) side;
@end

BarModel.m

#import "BarModel.h"

@interface BarModel ()

@property (nonatomic) int capacity;
@end

@implementation BarModel

- (instancetype)init
{
    self = [super init];
    if (self) {
        self.capacity = letterCapacity;
        _addedLetters = [[NSMutableArray alloc] init];
    }
    return self;
}

//  We'll use automatic notifications for this example
+ (BOOL)automaticallyNotifiesObserversForKey:(NSString *)key
{
    if ([key isEqualToString:@"arrayLetter"]) {
        return YES;
    }
    return [super automaticallyNotifiesObserversForKey:key];
}

- (BOOL) addLetter: (Letter*) letter onSide: (BarModelSide) side{
    if([_addedLetters count] > _capacity){
        return FALSE;
    }

    switch (side) {
        case BarModelSideLeft:
            [_addedLetters insertObject:letter atIndex:0];
            return TRUE;
            break;
        case BarModelSideRight:
            [_addedLetters addObject:letter];
            return TRUE;
            break;

        default:
            return FALSE;
            break;
    }
}

//  These methods enable KVC compliance
- (void)insertObject:(id)object inDataAtIndex:(NSUInteger)index
{
    self.addedLetters[index] = object;
}

- (void)removeObjectFromDataAtIndex:(NSUInteger)index
{
    [self.addedLetters removeObjectAtIndex:index];
}

- (id)objectInDataAtIndex:(NSUInteger)index
{
    return self.addedLetters[index];
}

- (NSArray *)dataAtIndexes:(NSIndexSet *)indexes
{
    return [self.addedLetters objectsAtIndexes:indexes];
}

- (NSUInteger)countOfData
{
    return [self.addedLetters count];
}
@end

BarView.h

#import <SpriteKit/SpriteKit.h>
#import "BarModel.h"

@interface BarView : SKSpriteNode

@property (nonatomic, strong) BarModel *barModel;

@end

BarView.m

#import "BarView.h"

@implementation BarView

static char MyObservationContext;

- (instancetype)init
{
    self = [super init];
    if (self) {
        //_barModel = [[BarModel alloc] init];

    }
    return self;
}

-(void)setBarModel:(BarModel *)barModel{

    if(_barModel != barModel)
        _barModel = barModel;

    [_barModel addObserver:self
                forKeyPath:@"arrayLetter"
                   options:(NSKeyValueObservingOptionOld | NSKeyValueObservingOptionNew)
                   context:&MyObservationContext];
}

- (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary *)change context:(void *)context
{
    //  Check if our class, rather than superclass or someone else, added as observer
    if (context == &MyObservationContext) {
        //  Check that the key path is what we want
        if ([keyPath isEqualToString:@"arrayLetter"]) {
            //  Verify we're observing the correct object
            if (object == self.barModel) {
                [self draw:change];
            }
        }
    }
    else {
        //  Otherwise, call up to superclass implementation
        [super observeValueForKeyPath:keyPath ofObject:object change:change context:context];
    }
}

- (void) draw: (NSDictionary*) change{
    NSLog(@"KVO for our container property, change dictionary is %@", change);
}
@end

When I ru this I get this "error":

2014-08-31 00:23:02.828 Testing[329:60b] Added letter
2014-08-31 00:23:02.830 Testing[329:60b] An instance 0x17803d340 of class BarModel was deallocated while key value observers were still registered with it. Observation info was leaked, and may even become mistakenly attached to some other object. Set a breakpoint on NSKVODeallocateBreak to stop here in the debugger. Here's the current observation info:
<NSKeyValueObservationInfo 0x17804eb50> (
<NSKeyValueObservance 0x1780cf180: Observer: 0x178111670, Key path: arrayLetter, Options: <New: YES, Old: YES, Prior: NO> Context: 0x100101428, Property: 0x17804eb80>

I tried to follow the instructions in error but can not find where to set break point. Please help me figure this out!

Answer 1

There are multiple problems with the code. In addition to what Tom Harrington said with respect to the specific error that was logged about failing to remove the observation:

You implemented the indexed collection accessors for a (non-existent) property named "data". That is, they have "Data" in their name where the property name should be.

The indexed collection property is addedLetters. So, the indexed collection mutating accessors should be:

- (void)insertObject:(id)object inAddedLettersAtIndex:(NSUInteger)index;
- (void)removeObjectFromAddedLettersAtIndex:(NSUInteger)index;

You don't really need the non-mutating accessors, since you have an array-type public property with a normal getter (i.e. -addedLetters).

By the way, that property is of type NSMutableArray which it should not be. The property should be of type NSArray, backed by an instance variable of type NSMutableArray. That is, the mutability of the type (as opposed to the property) should not be exposed through the public interface. When you do this, you have to manually declare the instance variable (since it should differ from the type of the property and auto-synthesis will get it wrong), make the property copy instead of strong, and implement the setter yourself to do a mutable copy of the passed-in immutable array:

- (void) setAddedLetters:(NSArray*)addedLetters
{
    if (addedLetters != _addedLetters)
        _addedLetters = [addedLetters mutableCopy];
}

Once you have implemented the indexed collection mutating accessors with the correct names, you must use only those methods to mutate the collection (after initialization). In particular, your -addLetter:onSide: method must not directly operate on the _addedLetters instance variable. This is the part that makes the class KVO-compliant for that property. The mere presence of the indexed collection mutating accessors does not help. They must be used for all actual mutations.

Your implementation of +automaticallyNotifiesObserversForKey: is redundant. Automatic notification is the default.

The BarView class is key-value observing a key path "arrayLetter" on its _barModel object, but that's not the name of the property on BarModel. I suppose you meant to use the key path "addedLetters".

Finally, for proper adherence to MVC design, your view should not have a reference to your model. It should have a reference to the controller. The controller can reflect the model to the view (or, in theory, adapt a model of a different internal design to what the view expects). Or, in a more traditional non-KVO approach, the controller would actually tell the view when something has changed and give it the updated data it should show.

So, your controller could expose its own addedLetters property:

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

It could be implemented as a derived property, forwarded through to the _barModel object:

+ (NSSet*)keyPathsForValuesAffectingAddedLetters
{
    return [NSSet setWithObject:@"barModel.addedLetters"];
}
- (NSArray*)addedLetters
{
    return self.barModel.addedLetters;
}

Then, the view would have a reference to the controller and not the model, and it would key-value observe the "addedLetters" key path on the controller.

Answer 2

The error is pretty descriptive. You add self as an observer of a BarModel object. At some point that object gets deallocated. But you never remove self as an observer by calling removeObserver:forKeyPath:context:. You need to do that.

First, in setBarModel, make sure to remove self as an observer of the previous value of _barModel.

Next, you probably need to add a dealloc method that does the same thing.