macOS GDC: grade my code

[MrFusion]

Hi everyone

Since installing Snow Leopard a few weeks ago, I have been messing around with GCD.
In this function, I am passing a sets of files to be read. The function returns immediately to avoid blocking the GUI.
Each file is read in a synchronous queue loop, whereby the amount of progress is reported back to the delegate (i.e. the GUI) after each file has been dealt with. Maybe I should include here an option to cancel.
The order in which the files are processed can not be predicted, but every file is processed before reporting to the GUI that everything has finished.

Do you think I got everything right in this code? Is it correct, or did I fall into some (hidden) traps of concurrency?

-(void) rawDataFromFiles:(NSArray *)files error:(NSError *)outError {
	dispatch_queue_t a_queue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_LOW, 0);
	
	dispatch_async(a_queue,^{ //don't wait for me
		//NSMutableArray *results = [NSMutableArray arrayWithCapacity:[files count]];
		//__block NSMutableArray *results = [[NSMutableArray alloc] init]; //__block prevents retain++ of results in block -> crash
		NSMutableArray *results = [[NSMutableArray alloc] initWithCapacity:[files count]];
		__block int progress = 0;
		void (^my_loop)(size_t) = ^(size_t i){ //what to do within loop
#warning todo: replace next line by file reading code 			
			[results addObject:[NSNumber numberWithInt:i]]; //todo read data 
			dispatch_sync(dispatch_get_main_queue(),^{ //report progress, wait for me, run on main thread
				[delegate importProgression:progress++ userInfo:nil];
			});
		}; 

		dispatch_apply([files count], a_queue, my_loop); //wait for me 
		
		dispatch_async(dispatch_get_main_queue(),^{ //finishing up, don't wait for me, run on main thread
			[delegate importDidFinish:[NSDictionary dictionaryWithObject:results forKey:@"results"]];
		});
		
		[results release];
	});	
        outError = nil;
}

 

[Catfish_Man]

Overall, looks fine. However, I do have an alternate approach that you might want to consider:

-(void) rawDataFromFiles:(NSArray *)files handlingResultOnQueue:(dispatch_queue_t)resultQueue withBlock:((^)(NSURL *,NSData *, NSError *))handler {	
	dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0),^{ //don't wait for me
		dispatch_group_t resultGroup = dispatch_group_create();
		void (^my_loop)(NSURL *, NSUInteger, BOOL *) = ^(NSURL *file, NSUInteger idx, BOOL *stop){ //what to do within loop
			NSError *err = nil;
#warning todo: replace next line by file reading code 			
			NSData *aResult = [something fileDataAtURL:file error:&err];
			dispatch_group_async(resultGroup, resultQueue,^{ //report back with the data we just read
				handler(file, aResult, err); //probably object lifetime issues, I haven't thought this through
			});
		}; 

		[files enumerateObjectsWithOptions:NSEnumerationConcurrent usingBlock:my_loop]; //equivalent to dispatch_apply
		
		//wait until we're done processing results. Not totally clear this is necessary.
		//removing it would change the semantics from "wait until results are processed before saying we're done" to
		//"wait until all results are in the queue for reporting before saying we're done"
		dispatch_group_wait(resultGroup); 
		
		dispatch_async(resultQueue,^{ //finishing up, don't wait for me
			[delegate importDidFinish:[NSDictionary dictionaryWithObject:results forKey:@"results"]];
		});
	});
}

This has a number of potential advantages:
*By processing results on a queue of the API clients' choice, you can make it easier to stay off the main queue (good for UI latency)
*By handing the data off to the result handler immediately, you can overlap reading and processing
*The NSError usage is... rather nonstandard, which is unfortunate, but it does nicely let you handle errors in a very fine-grained way.

 

[MrFusion]

Overall, looks fine. However, I do have an alternate approach that you might want to consider:

-(void) rawDataFromFiles:(NSArray *)files handlingResultOnQueue:(dispatch_queue_t)resultQueue withBlock:((^)(NSURL *,NSData *, NSError *))handler {	
	dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0),^{ //don't wait for me
		dispatch_group_t resultGroup = dispatch_group_create();
		void (^my_loop)(NSURL *, NSUInteger, BOOL *) = ^(NSURL *file, NSUInteger idx, BOOL *stop){ //what to do within loop
			NSError *err = nil;
#warning todo: replace next line by file reading code 			
			NSData *aResult = [something fileDataAtURL:file error:&err];
			dispatch_group_async(resultGroup, resultQueue,^{ //report back with the data we just read
				handler(file, aResult, err); //probably object lifetime issues, I haven't thought this through
			});
		}; 

		[files enumerateObjectsWithOptions:NSEnumerationConcurrent usingBlock:my_loop]; //equivalent to dispatch_apply
		
		//wait until we're done processing results. Not totally clear this is necessary.
		//removing it would change the semantics from "wait until results are processed before saying we're done" to
		//"wait until all results are in the queue for reporting before saying we're done"
		dispatch_group_wait(resultGroup); 
		
		dispatch_async(resultQueue,^{ //finishing up, don't wait for me
			[delegate importDidFinish:[NSDictionary dictionaryWithObject:results forKey:@"results"]];
		});
	});
}

That is so much better than what I had. Thanks!

This has a number of potential advantages:
*By processing results on a queue of the API clients' choice, you can make it easier to stay off the main queue (good for UI latency)
*By handing the data off to the result handler immediately, you can overlap reading and processing

You are absolutely right.

*The NSError usage is... rather nonstandard, which is unfortunate, but it does nicely let you handle errors in a very fine-grained way.

Oh, I figured I let the user know which files could not be read. What is the best way to do this?


Edit:
//wait until we're done processing results. Not totally clear this is necessary.

The data should be completely processed before it can be shown to the user. Since this takes some time, the UI starts beachballing. With GCD, this will hopefully not be the case.