Own implementation of OperationQueue executes before event is added

Question

I know I am going to get the "Should have used OperationQueues" but I had issues with it on the original implementation.. and infact was getting the same error as this. Both have confused me but this one even more so.

I have an add method which is syncronized on the operations and the worker is syncronized on that too.

The issues is that the objects getting picked off (at least to my knowledge) shouldn't be on the list yet. It then calls execute on the same one twice and skips the first one.

So if A B C was added. It might execute A C C and B doesnt even make an appearance. No other class can see operations and the only thing that adds to it is addOperation

Some output from the program.

[Queue Manager] Adding new operation [ 111 ] is <SendOperation: 0x1dd5c510>
[Queue Manager] executing operation [ 112 ]  is <SendOperation: 0x1dd5c510>
[Operation]  executing [ 112 ]
[Queue Manager] Adding new operation [ 112 ] is <SendOperation: 0x1dd266e0>
[Queue Manager] executing operation [ 112 ]  is <SendOperation: 0x1dd266e0>

It gets the address right. But the number wrong. The number is just an int in the Operation object. It is only ever sen on construction. It has not setters, or way of been modified.

What's going on here?

Looks like somehow im duplicating it in to the address of another one?

#import "MyOperationQueue.h"
#import "Operation.h"
@implementation MyOperationQueue

NSMutableArray *operations;
NSCondition *condition;

-(id) init {
    self = [super init];
    if(self != nil) {
        operations = [[NSMutableArray alloc] init];
        condition = [[NSCondition alloc] init];
    }
    return self;
}

- (void) start{
    [self performSelectorInBackground:@selector(run) withObject:self];
}

-(void)run{
    while(YES){

        [condition lock];
        [condition wait];

        NSMutableArray *buffer = [[NSMutableArray alloc] init];
        @synchronized(operations){
            while([operations count] != 0 ) {
                Operation *operation = [operations objectAtIndex:0];
                [operations removeObject:operation];

                NSLog(@"[Queue Manager] executing operation %i is %@",[operation getNumber],operation);
                [operation execute];
            }
        }
        [condition unlock];
    }

}

-(void)addOperation:(id)operation{
    @synchronized(operations){ 
        NSLog(@"[Queue Manager] Adding new operation [ %i ] is %@",[operation getNumber],operation);
        [operations addObject:(operation)];
    }
    [sendCondition signal];
}

@end


@implementation Operation

int idNumber;

-(id) initWithIdNumber:(int)idNumber_{
    self = [super init];
    if( self ) {
        idNumber = idNumber_;
    }
    return self;
}


- (void) execute{
    NSLog(@"[Operation]  executing [ %i ]",idNumber);
}

-(int) getIdNumber{
    return idNumber;
}

@end

Answer 1

Here's the beginning of your Operation implementation:

@implementation Operation

int idNumber;

If you think idNumber is an instance variable, you are mistaken. You have declared a global variable named idNumber. For more details, please read this answer. This mistake is described as “Option 2”.

If you want to declare a (private) instance variable, do it like this:

@implementation Operation {
    int _idNumber; // The _ prefix is a common Objective-C convention.
}

It would be even better to use a property, which you declare in your @interface like this:

@interface Operation: NSObject

@property (nonatomic, readonly) int idNumber;

If you declare it as a readonly property like this,

• the compiler generates the instance variable _idNumber for you, so you can remove your instance variable declaration, and
• the compiler generates a getter method named idNumber for you, so you can remove your getIdNumber method.

You made the same error with your operations and condition variables in your @implementation MyOperationQueue.