5
\$\begingroup\$

I recently interviewed with a company that needed a C/C++ programmer to work on the iOS side of the products. The job description indicated they needed someone with 4 years of Objective-C and iOS programming and I was surprised that they wanted to interview me.

Prior to this coding challenge I have never worked in Xcode, or programmed in iOS, Objective-c or swift. I am an absolute beginner in these areas. I still don't think I know these programming environments but I am learning.

Environtment

  • OSX - El Capitan
  • Xcode - Version 8.2 (8C38) // swift 3
  • Running in the iPhone 7 simulator.
  • Late 2010 17 inch MacBook Pro

The following section is the extraction of the email the hiring manager sent me:

Programming Challenge:

Create a static library or iOS Framework using Objective-C that performs the following 3 functions:

  • Collects the GPS location (latitude and longitude) of the user at a point in time
  • Collects the battery state and returns whether or not the device is plugged in and what percentage of life is left.
  • Accesses any publicly available, free API to collect the data of your choice and returns it (this should be a network call)

Build a simple application with 3 buttons and a label where text can be displayed. Each button should call into the three functions of the library described above and output the response to the label. Your application should consist of two tabs, one written in Objective-C and one written in Swift. Both tabs should call into the same Objective-C library and perform the same function.

Only use Apple frameworks to complete this task. Fully comment your code explaining your logic and choices where multiple choices are available. For example, Apple provides numerous ways to retrieve a network resource, document why you choose the solution you did.

Please send me the full SINGLE Xcode project.

End of Challenge

This question has been divided into 2 parts based on the size of the code to be reviewed. One part contains the Objective-C static library and the other part contains the simple application. This question contains the static library written in Objective-C the application can be found in this Question.

The source code and project files for both questions can be found at this GitHub repository in case you are interested in building it and running it. https://github.com/pacmaninbw/iOSCodeChallenge

The static library took 28 hours to research and code.

What I Desire From the Review

Since this is the first time I've programmed for iOS in both Objective-C and Swift I'd like to know:

  • Are there any memory leaks?
  • What iOS, Objective-C or Swift programming conventions have I mised or used incorrectly?
  • What are the obvious things that I should know that I don't?
  • What error checking should I have included that I didn't?
  • How could I have written this with less code?

PCI7DataModelLibrary.h

//
//  PCI7DataModelLibrary.h
//  PCI7DataModelLibrary
//
//  Created by Paul Chernick on 4/18/17.
//

/*
 * This file contains the API for the PCI7DataModelLibrary
 */

#import <Foundation/Foundation.h>
#import <UIKit/UIKit.h>

@interface PCI7DataModelLibrary : NSObject

- (id)init;
- (BOOL)IsGpsAvailable;
- (NSString *)provideGPSLocationData;
- (NSString *)provideBatteryLevelAndState;
- (NSString *)provideNetworkAccessData;
- (UIAlertController*)provideGPSAlerters;

@end

PCI7DataModelLibrary.m

//
//  PCI7DataModelLibrary.m
//  PCI7DataModelLibrary
//
//  Created by Paul Chernick on 4/18/17.
//

// This object provides a library interface to GPS data, battery data and a network service.
// The library consists of three different data models, one for the GPS data, one for the battery data and one
// for the network data. Each of the different data models is contained within it's own class. This library was
// implemented this way to ease the implementation, ease debugging, and allow multiple engineers to work in
// parallel to implement the library. This implementation follows what is sometimes know as the Single
// Responsibility Principle.
//
// Each child data model has it's own initialization functions and is completely self contained. There are no
// dependencies between the different data models.

#import "PCI7DataModelLibrary.h"
#import "PCIGpsDataModel.h"
#import "PCIBatteryDataModel.h"
#import "PCINetworkingDataModel.h"

// TODO : create a tabel of data models and symbolic constants that provide indexes into that table.
@implementation PCI7DataModelLibrary
{
    PCIBatteryDataModel *batteryDataModel;
    PCIGpsDataModel *gpsDataModel;
    PCINetworkingDataModel *networkDataModel;
    BOOL unableToEstableConnection;
}

- (BOOL)IsGpsAvailable {
    BOOL GpsIsAvailble = NO;

    if (gpsDataModel) {
        GpsIsAvailble = gpsDataModel.doesGPSHardWareExists;
    }

    return GpsIsAvailble;
}

- (UIAlertController*)provideGPSAlerters {
    UIAlertController* gpsAlertController = nil;
    if (gpsDataModel) {
        gpsAlertController = gpsDataModel.alertUserNoGPSHardware;
    }

    return gpsAlertController;
}

- (NSString *)provideGPSLocationData {
    NSString *gpsLocationData = nil;

    if (gpsDataModel) {
        gpsLocationData = [gpsDataModel provideGPSLongitudeAndLatitudeWithTimeStamp];
    }
    else {
        gpsLocationData = @"Unable to access location data at this time.";
    }

    return gpsLocationData;
}

- (NSString *)provideBatteryLevelAndState {
    NSString *batteryLevelAndState = nil;

    if (batteryDataModel) {
        batteryLevelAndState = batteryDataModel.provideBatteryLevelAndState;
    }
    else {
        batteryLevelAndState = @"Unable to access battery state and level at this time";
    }

    return batteryLevelAndState;
}

- (NSString *)provideNetworkAccessData {
    NSString *networkAccessData = nil;

    if (networkDataModel) {
        networkAccessData = networkDataModel.provideANetworkAccess;
    }
    else {
        // If an attemp to create the
        if (unableToEstableConnection) {
            networkAccessData = @"Unable to establish network connection";
        }
        else {
            networkAccessData = @"provideANetworkAccess Not Implemented Yet";
        }
    }

    return networkAccessData;
}

- (id)init {
    if (self = [super init]) {
        batteryDataModel = [[PCIBatteryDataModel alloc] init];
        gpsDataModel = [[PCIGpsDataModel alloc] init];
        networkDataModel = [[PCINetworkingDataModel alloc] init];
        // TODO : add error handling for failure of any of the child initialization.
        // This includes handling memory allocation errors, device errors and
        // networking errors. Catch all lower level exceptions here
        // return nil if error is uncorrectible.
        if (!networkDataModel) {
            //  Don't report errors at this point, report errors when the user clicks the button.
            unableToEstableConnection = YES;
        }
        // If none of the data models can be constructed then this object has no meaning.
        // If there are more than the 3 data models create a table of data models and loop
        // through all of them to check if any of the data models are created.
        if ((!batteryDataModel) && (!gpsDataModel) && (!networkDataModel)) {
            return  nil;
        }
    }

    return  self;
}

@end

PCIGpsDataModel.h

//
//  PCIGpsDataModel.h
//  PCIDataModelLibrary
//

//  The GPSDataModel class is responsible for interfacing with the CLLocationManager to retrieve the
//  GPS Latitude and Longitude.

#import <Foundation/Foundation.h>

@interface PCIGpsDataModel : NSObject

- (id)init;
- (BOOL)doesGPSHardWareExists;
- (NSString*)provideGPSLongitudeAndLatitudeWithTimeStamp;
- (UIAlertController*)alertUserNoGPSHardware;

@end

PCIGpsDataModel.m

//
//  PCIGpsDataModel.m
//  PCIDataModelLibrary
//

#import <CoreLocation/CoreLocation.h>
#import <UIKit/UIKit.h>
#import "PCIGpsDataModel.h"


@interface PCIGpsDataModel () <CLLocationManagerDelegate>

@property (nonatomic, strong) CLLocationManager *locationManager;
@property (nonatomic, strong) NSDateFormatter *dateFormatter;

- (void)initializeGPSHardwareExists;
- (NSString*)dateFromLocation;
- (NSString*)latitudeFromLocation;
- (NSString*)longitudeFromLocation;

@end

@implementation PCIGpsDataModel
{
    BOOL mustUseGPSHardware;
    BOOL hardwareExistsOnDevice;
    BOOL firstTimeGPSDataRequested;         // Flag to allow asking user to use alternate method rather than GPS and to enable location service
    CLLocation *lastReportedLocation;
}

- (NSDateFormatter *)dateFormatter {
    if (_dateFormatter == nil) {
        _dateFormatter = [[NSDateFormatter alloc] init];
        [_dateFormatter setDateStyle:NSDateFormatterMediumStyle];
        [_dateFormatter setTimeStyle:NSDateFormatterLongStyle];
    }
    return _dateFormatter;
}

#pragma mark - Private Functions

- (void)initializeGPSHardwareExists {
    if (lastReportedLocation) {
        // The GPS hardware provides better accuracy than WiFi or Radio triangulation.
        // Both horizontal and vertical accuracy will be greater than zero if the GPS hardware is available.
        if (([lastReportedLocation horizontalAccuracy] > 0) && ([lastReportedLocation verticalAccuracy] > 0)) {
            hardwareExistsOnDevice = YES;
        }
        else {
            hardwareExistsOnDevice = NO;
        }

    }
    else {
        hardwareExistsOnDevice = NO;
    }
}

- (NSString*)dateFromLocation {
    NSString* dateString = nil;

    if (lastReportedLocation) {
        NSDate* timeStampInDateForm = [lastReportedLocation timestamp];
        dateString = [self.dateFormatter stringFromDate:timeStampInDateForm];
    }
    else {
        dateString = @"No location data.";
    }

    return dateString;
}

- (NSString*)latitudeFromLocation {
    NSString* latitude = nil;

    if (lastReportedLocation) {
        CGFloat untranslatedLatitude = lastReportedLocation.coordinate.latitude;
        NSString* direction = @"North";

        if (untranslatedLatitude < 0.0) {
            direction = @"South";
        }

        latitude = [NSString stringWithFormat:@"Latitude = %4.2f %@", fabs(untranslatedLatitude), direction];
    }
    else {
        latitude = @"No location data.";
    }

    return latitude;
}

- (NSString*)longitudeFromLocation {
    NSString* longitude = nil;

    if (lastReportedLocation) {
        CGFloat untranslatedLongitude = lastReportedLocation.coordinate.longitude;
        NSString* direction = @"East";

        if (untranslatedLongitude < 0.0) {
            direction = @"West";
        }
        longitude = [NSString stringWithFormat:@"Longitude = %4.2f %@", fabs(untranslatedLongitude), direction];
    }
    else {
        longitude = @"No location data.";
    }

    return longitude;
}

#pragma mark - Location Manager Interactions

/*
 * Callback function, the CLLocationManager calls this function with updated location data when changes to the location occur.
 */
- (void)locationManager:(CLLocationManager *)manager didUpdateToLocation:(CLLocation *)newLocation fromLocation:(CLLocation *)oldLocation {
    // test that the horizontal accuracy does not indicate an invalid measurement
    if (([newLocation horizontalAccuracy] > 0) && ([newLocation verticalAccuracy] > 0)) {
        hardwareExistsOnDevice = YES;
    }
    else {
        hardwareExistsOnDevice = NO;
    }

    lastReportedLocation = newLocation;
    [self initializeGPSHardwareExists];

}

- (BOOL)doesGPSHardWareExists {
    return hardwareExistsOnDevice;
}

#pragma mark - public interfaces

/*
 * This alert is generated here in the library so that the user interface doesn't need
 * to know about the business logic. The yes/no buttons affect variables here in the
 * library that should not be exposed to the user interface.
 */
- (UIAlertController*)alertUserNoGPSHardware {
    [NSThread isMainThread];
    UIAlertController *alertToPresent = nil;
    NSString* alertTitleString = @"GPS Alert";
    NSString* alertMessage = @"No GPS hardware use Triangulation?";

    if (!hardwareExistsOnDevice && mustUseGPSHardware) {
        alertToPresent = [UIAlertController alertControllerWithTitle: alertTitleString message:alertMessage
                preferredStyle:UIAlertControllerStyleAlert];
        UIAlertAction* yesButton = [UIAlertAction actionWithTitle:@"YES" style:UIAlertActionStyleDefault
                handler:^(UIAlertAction * action) {mustUseGPSHardware = NO;}];
        [alertToPresent addAction:yesButton];
        UIAlertAction* noButton = [UIAlertAction actionWithTitle:@"NO" style:UIAlertActionStyleDefault
                handler:^(UIAlertAction * action) {mustUseGPSHardware = YES;}];
        [alertToPresent addAction:noButton];
    }

    return alertToPresent;
}

- (NSString*)provideGPSLongitudeAndLatitudeWithTimeStamp {
    NSString *gpsLongitudeAndLatitudeWithTimeStamp = nil;

    if (!lastReportedLocation) {
        gpsLongitudeAndLatitudeWithTimeStamp = @"No Location data available";
    }
    else {
        if (hardwareExistsOnDevice || !mustUseGPSHardware) {
            gpsLongitudeAndLatitudeWithTimeStamp = [NSString stringWithFormat:@"%@\n%@\n time stamp = %@", self.latitudeFromLocation, self.longitudeFromLocation, self.dateFromLocation];
        }
        else {
            gpsLongitudeAndLatitudeWithTimeStamp = [NSString
                stringWithFormat:@"GPS hardware not on device using alternate method\n%@\n%@\n time stamp = %@",
                self.latitudeFromLocation, self.longitudeFromLocation, self.dateFromLocation];
        }
    }

    firstTimeGPSDataRequested = NO;

    return  gpsLongitudeAndLatitudeWithTimeStamp;
}

- (void)gpsSetMustUseGPSHardware {
    mustUseGPSHardware = YES;           // Not current checked.
}

- (void)gpsAllowWiFiOrRadioTriangulation {
    mustUseGPSHardware = NO;           // Not current checked.
}

- (void)gpsSetHardwareExists {
    hardwareExistsOnDevice = NO;        // This value may be changed on the first or following location updates.
}

// CLLocationManager returns locations based on the delegate model, Apple does not provide an alternate method. The locations are returned based
// on the device moving a specified distance. Requesting greater accuracy can for the service to use GPS hardware if it is available, it may
// default to WiFi or Radio triangulation. Using when the GPS hardware is used, it can affect power usage, this should be considered
// during implementation in real life.

// The CLLocation manager is configured and data collection is started during the initialization of this data model,
// I considered it a better than doing all this work during the first click of the button. The initialization will fail
// if the user clicks the "Don't Allow" button on the alert. This initialization will also failed if Info.plist does not
// contain the following
//      <key>NSLocationWhenInUseUsageDescription</key>
//      <string>This will be used to obtain your current location.</string>
//      <key>NSLocationAlwaysUsageDescription</key>
//      <string>This application requires location services to work</string>

- (id)init {
    if (self = [super init]) {
        mustUseGPSHardware = YES;
        firstTimeGPSDataRequested = YES;
        lastReportedLocation = nil;         // This is updated periodically, set to nil to prevent access to uknown address until update
        _locationManager = [[CLLocationManager alloc] init];
        if (_locationManager) {
            // Attempt to force the device to use the GPS rather than WiFi or Radio Triagulation
            self.locationManager.desiredAccuracy = [ @"AccuracyBest" doubleValue];
            // If the user moves 100 meters then a location update should occur.
            self.locationManager.distanceFilter = [@100.0 doubleValue];
            // The following code checks to see if the user has authorized GPS use.
            if ([self.locationManager respondsToSelector:@selector(requestWhenInUseAuthorization)]) {
                [self.locationManager requestWhenInUseAuthorization];
            }
            // Now that the configuration of the CLLocationManager has been completed start updating the location.
            [self.locationManager startUpdatingLocation];
        }
        self.locationManager.delegate = self;
    }


    return  self;
}

@end

PCIBatteryDataModel.h

//
//  PCIBatteryDataModel.h
//  PCIDataModelLibrary
//
//

// Provides public interfaces to get the battery level and battery state from the device.


#import <Foundation/Foundation.h>

@interface PCIBatteryDataModel : NSObject

- (id)init;
- (NSString *)provideBatteryLevelAndState;
- (NSString *)provideBatteryLevel;
- (NSString *)provideBatteryState;

@end

PCIBatteryDataModel.m

//
//  PCIBatteryDataModel.m
//  PCIDataModelLibrary
//

// Provides public interfaces to get the battery level and battery state from the device.

#import <UIKit/UIKit.h>
#import "PCIBatteryDataModel.h"

@implementation PCIBatteryDataModel
{
    UIDevice *thisDevice;
}

- (id)init {
    if (self = [super init]) {
        // To optimize performance of the calls to getBatteryLevelAndState, getBatterLevel, getBatteryState
        // get a pointer to the device only once and enable battery monitoring only once. Battery monitoring
        // must be enabled to get the information from the device or the simulator. The simulator does not
        // fully support modeling the battery.
        thisDevice = [UIDevice currentDevice];
        [thisDevice setBatteryMonitoringEnabled:YES];
    }
    return  self;
}

// each of the following functions could return [NSString stringWithFormat: FORMATSTRING, arguments], but
// the following functions/methods implementations allow for editing and improvements.
// getBatteryLevelAndState could have performed all of the operations, but I try to follow the Single
// Responsibility Principle as well as the KISS principle.

- (NSString *)provideBatteryLevelAndState {
    NSString *batteryStateAndLevel = nil;

    batteryStateAndLevel = [NSString stringWithFormat:@"%@\n%@", self.provideBatteryState, self.provideBatteryLevel];

    return batteryStateAndLevel;
}

- (NSString *)provideBatteryLevel {
    NSString *batteryLevelString = nil;

    batteryLevelString = [NSString stringWithFormat:@"Battery Level = %0.2f", [thisDevice batteryLevel]];

    return batteryLevelString;
}

- (NSString *)provideBatteryState
{
    NSString *batterStateString = nil;
    NSArray *batteryStateArray = @[
                                   @"Battery state is unknown",
                                   @"Battery is not plugged into a charging source",
                                   @"Battery is charging",
                                   @"Battery state is full"
                                   ];

    batterStateString = [NSString stringWithFormat:@"Battery state = %@", batteryStateArray[thisDevice.batteryState]];

    return batterStateString;

}

@end

PCINetworkingDataModel.h

//
//  PCINetworkingDataModel.h
//  PCIDataModelLibrary
//
//  Created by Paul Chernick on 4/17/17.

//  Provides an internet connection interface that retrieves the 15 minute delayed price of American Express.

#import <Foundation/Foundation.h>

@interface PCINetworkingDataModel : NSObject<NSURLSessionDataDelegate>

- (id)init;
- (NSString*)provideANetworkAccess;

@end

PCINetworkingDataModel.m

//
//  PCINetworkingDataModel.m
//  PCIDataModelLibrary

// If you decide to build and run this in the simulator please see this article on stackoverflow.com
// http://stackoverflow.com/questions/41273773/nw-host-stats-add-src-recv-too-small-received-24-expected-28
// the Xcode issue occurs in this code.
// You will also need the following in your Info.plist
//      <key>NSAppTransportSecurity</key>
//      <dict>
//          <key>NSAllowsArbitraryLoads</key>
//          <true/>
//      </dict>

/*  Provides an internet connection interface that retrieves the 15 minute delayed price of American Express.
 *  This model uses the Apple's NSURLSession Networking interfaces that are provided by the foundation and
 *  core foundation. 
 *
 *  Due to the nature of the data there is no reason to directly access sockets in the lower layers and Apple
 *  recommends using the highest interfaces possible. This app is written specifically for iOS devices such
 *  as the iPhone and iPad and doesn't need to be concerned with portability. The NSURLSession Networking
 *  interfaces are used because this only needs to access public data on the internet. The NSURLSession
 *  Networking interfaces provide the initial connection and reachabilty checking.
 *
 *  From https://developer.apple.com/library/content/documentation/NetworkingInternetWeb/Conceptual/NetworkingOverview/CommonPitfalls/CommonPitfalls.html#//apple_ref/doc/uid/TP40010220-CH4-SW1
 *      Sockets have many complexities that are handled for you by higher-level APIs. Thus, you will have
 *          to write more code, which usually means more bugs.
 *      In iOS, using sockets directly using POSIX functions or CFSocket does not automatically activate the
 *          device’s cellular modem or on-demand VPN.
 *      The most appropriate times to use sockets directly are when you are developing a cross-platform tool 
 *          or high-performance server software. In other circumstances, you typically should use a higher-level API.
 */

#import <Foundation/Foundation.h>
#import <CoreFoundation/CoreFoundation.h>
#import "PCINetworkingDataModel.h"

// Private interface and private functions.
@interface PCINetworkingDataModel()

@property (nonatomic,strong) NSURLSessionDataTask *retrieveDataStockPriceTask;
@property (nonatomic,strong) NSURLSession *retrieveDataPriceSession;

- (NSURLSession *)createSession;
- (void)URLSession:(NSURLSession *)session task:(NSURLSessionTask *)task didCompleteWithError:(NSError *)error;
- (void)URLSession:(NSURLSession *)session dataTask:(NSURLSessionDataTask *)dataTask didReceiveData:(NSData *)stockPriceData;
- (void)setupAndStartdownloadStockPrice;
- (NSString*)ParseDownoadedFileForStockPrice;
- (void)createStockPriceFullFileSpec;
- (NSString*)retrieveStringFromDownloadedFileByUrl;

@end

@implementation PCINetworkingDataModel
// Instance variables
{
    NSString* googleFinanceUrl;
    NSString* stockPriceFilePath;
    NSString* stockPriceFileName;
    NSString* stockPriceFileFullFileSpec;
    NSString* downloadFailedErrorString;
    BOOL downloadHadErrors;
//    BOOL downloadCompleted;
}

//  Create a session using the main operation queue rather than an alternate, this allows synchronous programming
//  rather than asynchronous programming. The session is resumed in the calling function.
- (NSURLSession *)createSession
{
    static NSURLSession *session = nil;
    session = [NSURLSession sessionWithConfiguration:[NSURLSessionConfiguration defaultSessionConfiguration]
            delegate:self
            delegateQueue:[NSOperationQueue mainQueue]];

    return session;
}

// Apple developer documentation indicates that this error handler should check reachability after a slight
// wait  (15 minutes) and then restart the download session. Since the original session start was based on
// a button click instead of checking reachability and then restarting the request the error will be reported
// and the task will quit.
// https://developer.apple.com/library/content/documentation/NetworkingInternetWeb/Conceptual/NetworkingOverview/WhyNetworkingIsHard/WhyNetworkingIsHard.html#//apple_ref/doc/uid/TP40010220-CH13-SW3
//  For requests made at the user’s behest:
//      Always attempt to make a connection. Do not attempt to guess whether network service is available, and do not cache that determination.
//      If a connection fails, use the SCNetworkReachability API to help diagnose the cause of the failure. Then:
//      If the connection failed because of a transient error, try making the connection again.
//      If the connection failed because the host is unreachable, wait for the SCNetworkReachability API to call your registered callback.
//          When the host becomes reachable again, your app should retry the connection attempt automatically without user intervention
//          (unless the user has taken some action to cancel the request, such as closing the browser window or clicking a cancel button).
//      Try to display connection status information in a non-modal way. However, if you must display an error dialog, be sure that it does
//          not interfere with your app’s ability to retry automatically when the remote host becomes reachable again. Dismiss the dialog
//          automatically when the host becomes reachable again.

- (void)URLSession:(NSURLSession *)session task:(NSURLSessionTask *)task didCompleteWithError:(NSError *)error{
    if (error) {
        downloadHadErrors = YES;
        NSString* errorString = [error localizedFailureReason];
        downloadFailedErrorString = [NSString stringWithFormat:@"An error occurred while downloading the stock price %@", errorString];
        NSLog(@"Data retrieval completed with error: %@\n", error);
        NSLog(@"Data retrieval completed with error: %@", errorString);
    }

    // The download failed, release any system resources used by the session, no need to finish any tasks here.
    [self.retrieveDataPriceSession invalidateAndCancel];
}

//  We got the stock price data from the web interface, save it to a file so that it can be processed later.
- (void)URLSession:(NSURLSession *)session dataTask:(NSURLSessionDataTask *)dataTask didReceiveData:(NSData *)stockPriceData {
    NSError *writeError;

    if (stockPriceData) {
        NSString *stockPriceRequestReply = [[NSString alloc] initWithData:stockPriceData encoding:NSUTF8StringEncoding];
        downloadHadErrors = NO;

        NSString* urlOfstockPriceFileFullFileSpec = [NSString stringWithFormat:@"file://%@", stockPriceFileFullFileSpec];
        NSURL *downloadedDataInFile = [NSURL URLWithString:urlOfstockPriceFileFullFileSpec];

        if (![stockPriceRequestReply  writeToURL:downloadedDataInFile atomically:NO encoding:NSUTF8StringEncoding error: &writeError]) {
            NSString* errorString = [writeError localizedFailureReason];
            downloadFailedErrorString = [NSString stringWithFormat:@"An error occurred while writing file %@", writeError];
            downloadHadErrors = YES;
        }
    }
    else {
        NSString* errorString = [writeError localizedFailureReason];
        downloadFailedErrorString = [NSString stringWithFormat:@"An error occurred while receiving the data %@", writeError];
        downloadHadErrors = YES;
    }

    // Since the download seems to have completed, finish any unfinished business and then invalidate the session
    // to release any resources that need to be released.
    [self.retrieveDataPriceSession invalidateAndCancel];

}

// Set up a delegate based HTTP download and start the download
- (void)setupAndStartdownloadStockPrice {
    downloadHadErrors = NO;
    downloadFailedErrorString = nil;
    // Set up a NSURLSession to do an HTTP Download from the google finance
    self.retrieveDataPriceSession = [self createSession];
    NSURL *stockPriceURL = [NSURL URLWithString:googleFinanceUrl];
    NSMutableURLRequest *request = [NSMutableURLRequest requestWithURL:stockPriceURL];

    self.retrieveDataStockPriceTask = [self.retrieveDataPriceSession dataTaskWithRequest:request];
    [self.retrieveDataStockPriceTask resume];

}

/*
 * The data returned and stored in the file looks like:
 // [
 {
 "id": "1033"
 ,"t" : "AXP"
 ,"e" : "NYSE"
 ,"l" : "80.11"
 ,"l_fix" : "80.11"
 ,"l_cur" : "80.11"
 ,"s": "0"
 ,"ltt":"3:30PM EDT"
 ,"lt" : "Apr 20, 3:30PM EDT"
 ,"lt_dts" : "2017-04-20T15:30:24Z"
 ,"c" : "+4.56"
 ,"c_fix" : "4.56"
 ,"cp" : "6.04"
 ,"cp_fix" : "6.04"
 ,"ccol" : "chg"
 ,"pcls_fix" : "75.55"
 }
 ]
 *
 * The only relavent data is ,"l" : "80.11"
 * The data needs to be parsed so that it only returns 80.11
 */
- (NSString*)retrieveStringFromDownloadedFileByUrl {
    NSString* fileContents = nil;
    NSString* urlOfstockPriceFileFullFileSpec = [NSString stringWithFormat:@"file://%@", stockPriceFileFullFileSpec];
    NSURL *downloadedFile = [NSURL URLWithString:urlOfstockPriceFileFullFileSpec];
    NSError *readErrors;

    fileContents = [[NSString alloc] initWithContentsOfURL:downloadedFile encoding:NSUTF8StringEncoding error:&readErrors];
    if (fileContents == nil) {
        downloadFailedErrorString = [readErrors localizedFailureReason];
        NSLog(@"Error reading file at %@\n%@", downloadedFile, [readErrors localizedFailureReason]);
        downloadHadErrors = YES;
        return downloadFailedErrorString;
    }
    else {
        NSRange range = [fileContents rangeOfString:@",\"l\" : \""];
        NSString *hackedFileContents = [fileContents substringFromIndex:NSMaxRange(range)];
        NSArray *hackedStringComponents = [hackedFileContents componentsSeparatedByString:@"\""];
        fileContents = hackedStringComponents[0];
    }

    return fileContents;
}

// Read the downloaded file into a string, then parse the string.
// This is implemented without a check that the download task completed. Since the main operation
// queue is being used rather than an alternate queue it is assumed that this program is
// synchronous rather than asynchronous
- (NSString*)ParseDownoadedFileForStockPrice {
    NSString* returnAmexStockPrice = nil;

    //  If there were errors, just report the errors, don't attempt to parse the data in the file
    //  since it may not be there or it may be incomplete.
    if (downloadHadErrors) {
        return downloadFailedErrorString;
    }

    returnAmexStockPrice = [self retrieveStringFromDownloadedFileByUrl];

    return returnAmexStockPrice;
}

//  Returns a string for the label in the user interface when the Networking button is clicked.
//  Attempt to connect using WiFi first because this uses less power than the cellphone (WWAN)
//  connection and does't add cost to the user (data charge on cell phone usage).
//  Download a file containing the current delayed American Express stock price. Parse the file
//  to get only the stock price. Embed the stock price within the text to be returned.
//
//  The download is not performed in the background, it is a download initiated when the user
//  clicks on the button. Any errors such as no connection, or the URL can't be reached are
//  therefore reported to the user.
- (NSString*)provideANetworkAccess {
    NSString* networkAccessDisplayString = @"Network access not implemented yet";

    [self setupAndStartdownloadStockPrice];

    // Since the download is implemented on the main queue this is a synchronous rather
    // than asynchronous program. It is therefore safe to process the data here rather
    // than in a delegate.
    NSString* amexStockPrice = [self ParseDownoadedFileForStockPrice];

    if (!downloadHadErrors) {
        NSString* preface = @"The current American Express stock price with a 15 minute delay is ";
        networkAccessDisplayString = [NSString stringWithFormat:@"%@ $%@", preface, amexStockPrice];
    }
    else {
        networkAccessDisplayString = amexStockPrice;
    }

    return networkAccessDisplayString;
}

- (void)createStockPriceFullFileSpec {
    // find a path to a writable directory to store the downloaded file. This is done once
    // so that button clicks are not affected by a directory search.
    NSArray *paths = NSSearchPathForDirectoriesInDomains(NSDocumentDirectory, NSUserDomainMask, YES);
    stockPriceFilePath = [paths objectAtIndex:0];
    stockPriceFileName = @"latestStockQuote.txt";
    stockPriceFileFullFileSpec = [stockPriceFilePath stringByAppendingPathComponent:stockPriceFileName];

}

// Simple setup since all networking actions will be performed when the user clicks a button in the
// application interface. The alternate to this would be to set up a timed loop that would download
// the information on a periodic basis in the background using the features of NSUrlSession (not an
// explict loop in this file).

// While implementing this as a loop would show the results of the in the background would allow the
// button click to perform more quickly, it would use more power, and if the user isn't close to a WiFi
// connection it would add more cost to the users data charges. Since the amount of data expected is
// less than 1K in size, it shouldn't take more than 5 seconds to download. If it was larger than 1K
// this would be implemented as a loop to repeated download the data in the background every 15 minutes.
- (id)init {
    googleFinanceUrl = @"http://finance.google.com/finance/info?client=ig&q=NSE:AXP";
    downloadHadErrors = NO;
    downloadFailedErrorString = nil;
    stockPriceFilePath = nil;
    stockPriceFileName = nil;
    stockPriceFileFullFileSpec = nil;

    [self createStockPriceFullFileSpec];

    return self;
}

@end
\$\endgroup\$
4
\$\begingroup\$

For having never done Objective-C before, this looks pretty good! Here are some things I would do differently.

Library Name

What is PCI7? For an acronym used in every class in your example, it would have been nice to have an explanation somewhere of what it means!

Data Model

In the header, I would not #import <UIKit/UIKit.h> for just 1 member function. I would forward declare UIAlertController and then do the #import in the source file. The reason I'd do it that way is that any file that imports this header will then also import UIKit.h which is a fairly large header to import.

I would also use more Objective-C style naming. This is something you'll see more of as you use Objective-C more often. I'd make IsGpsAvailable have a lowercase first letter and capitalize all of GPS like in the other method names. I'd probably also make most of those methods into properties. But those are minor decisions.

More importantly, I'd have the library return something more useful than strings. Strings are pain to work with for stuff that's not textual. For the GPS location data, I'd return a CLLocation*, since that's what most other methods that deal with locations take, or at least a CLLocationCoordinate2D if that would be more appropriate. The caller can decide if they want to display it as a string or do a calculation with it, or pass it to another method. Likewise with the battery state and the stock price.

Lastly, I'd advise you to make your BOOLs positive rather than negative. It becomes confusing to understand otherwise. Was it not able to establish the connection? Yes it was not able to do that. Wait… what?

I'd make it look like this:

#import <Foundation/Foundation.h>

@class UIAlertController;

@interface PCI7DataModelLibrary : NSObject
    @property (readonly) BOOL isGPSAvailable;
    @property (readonly) CLLocation* GPSLocationData;
    @property (readonly) UIDeviceBatteryState* batteryLevelAndState;
    @property (readonly) NSString* networkAccessData;
    @property (readonly) BOOL connectionEstablished;

    -(id)init;
    -(UIAlertController*)GPSAlerters;
@end

That's all I have time for tonight.

\$\endgroup\$
  • 1
    \$\begingroup\$ PCI7 -PC are my Initials, I is the first letter of the company name I interviewed with, 7 - the seventh attempt to merge the library into the Xcode project. \$\endgroup\$ – pacmaninbw May 10 '17 at 12:30

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.