Top wiki pages as an app

I recently did an interview task. I was rejected because of bad code quality. There was two tasks. Here I will present the first, second will be posted in time.

This project is avalible on GitHub

First: Table view

Please create an iPhone application that will present the list of most popular wikis from wikia.com

Requirements

• The application should include one screen using UITableView

• Each row in the table should contain: wiki title, wiki url, wiki thumbnail image

• API calls should not block the UI

Notes

• UI Code should be done in code, no .xib or .storyboard files

• The application can have additional screens

• The application should be "release ready" (shall not use any private API, etc.)

• The applicant can use any library that will help him/her in achieving the expected result

• Please implement this in Objective-C only

WikiApi.h (5 lines)

#import 

@interface WikiApi : NSObject

+(void)fetchTop10WithThumbnailDownload:(void (^)(NSString *key,UIImage *thumbnail))onThumbnailDownload complete:(void (^)(NSDictionary* json))complete;
+(void)fetchThumbnail:(NSString*)link complete:(void (^)(UIImage *thumbnail))complete;
@end

WikiApi.m (106 lines)

```
#import "WikiApi.h"

@implementation WikiApi

+(void)fetchTop10WithThumbnailDownload:(void (^)(NSString key,UIImage thumbnail))onThumbnailDownload complete:(void (^)(NSDictionary* json))complete {
NSURL *url = [[NSURL alloc]initWithString:@"http://www.wikia.com/wikia.php?controller=WikisApi&method=getList&lang=en&limit=10"];
NSURLRequest*request = [[NSURLRequest alloc]initWithURL:url];
//Request list of wikis
[[[NSURLSession sharedSession] dataTaskWithRequest:request completionHandler:^(NSData data, NSURLResponse response, NSError *error) {
if (error != NULL) {
NSLog(@"%@",[error localizedDescription]);
return;
}
NSDictionary *json = [NSJSONSerialization JSONObjectWithData:data options:0 error:&error];

//Gett

The first thing I would have done in accomplishing this task is probably something very big that really cost you.


Each row in the table should contain: wiki title, wiki url, wiki thumbnail image

One of the absolute first things I would have done would be to create a class to represent the "wiki" object.

@interface WikiPage

@property NSNumber *id;  
@property NSString *title;
@property NSURL *url;
@property UIImage *thumbnail; 

@end

We then create an array of these objects for our table view's data source to deal with.

As for the code you actually wrote, I want to just comment on your WikiApi class for now.

It's not a particularly good sign that we've implemented only classs methods. Instead, we should make an instantiable WikiPageFetcher perhaps. And rather than passing blocks, let's set up a delegate. For that, we need a protocol. Something like this:

@protocol WikiPageFetcherDelegate 

@required - (void)wikiFetcher:(WikiPageFetcher *) wikiFetcher didFetchWikiPage:(WikiPage *)wikiPage;

@optional - (void)wikiFetcher:(WikiPageFetcher *) wikiFetcher didFailWithError:(NSError *)error;

@end

You'll notice here that we're going to pass a WikiPage object one at a time. Each time we finish parsing and downloading the thumbnail for an individual page, we're going to pass it back. By doing this, we can actually dynamically update our table view and add the results one at a time as they come in instead of waiting for the whole thing to complete.

Additionally, we could add a whole separate protocol method for the completion of the thumbnail download. That way we can quickly inform the delegate that we've parsed the link (shouldn't take long) and come back a few seconds later to pass the image back.

Importantly, internally, we'll get our code passed out to multiple threads. Each of the top 10 (or however many) wiki links we're grabbing will be handled by its own thread, so we should be able to get our results slightly quicker.

It's not a good sign that we've hard coded the number of results we're requesting. Yes, they specified 10... but you'll have specifications like this throughout your career. And then the specification will change. So make the code more versatile and prepare ahead to be asked for a different number.

I think this is enough to get you started on a revision before I go into too much more specific detail.

The gist of this is, my end usage should look something like this:

@interface MyTableViewController() 

@property NSMutableArray *wikiPages;
@property WikiPageFetcher *wikiFetcher;

@end

@implementation MyTableViewController

- (void)viewDidLoad {
    [super viewDidLoad];
    self.wikiPages = [NSMutableArray array];
    self.wikiFetcher = [WikiPageFetcher wikiPageFetcherWithDelegate:self];
}

- (void)viewDidAppear {
    [super viewDidAppear];
    [self refresh];
}

- (void)refresh {
    [self.wikiFetcher loadTopWikiPages:10];
}

- (void)wikiFetcher:(WikiPageFetcher *)wikiFetcher 
   didFetchWikiPage:(WikiPage *)wikiPage {
    [self.wikiPages addObject:wikiPage];
    NSIndexPath *indexPath = [NSIndexPath indexPathForRow:(self.wikiPages.count - 1) 
                                                inSection:0];
    [self.tableView insertRowsAtIndexPaths:@[indexPath] 
                          withRowAnimation:UITableViewRowAnimationAutomatic];
}

I consider it important to separate out the refresh method, because you may want to implement a pull-to-refresh or let the user refresh in other ways, or refresh automatically every 5 minutes, etc., so this is just good practice for all sorts of other applications. This is particularly true when refresh does more than just a single method call.

Once you can get things to this point, you'll be ready for another round of reviews.

Make sure that the delegate methods are called on the main thread but all the downloading stuff remains in the background.

StackExchange Code Review Q#93001, answer score: 5