# Archiving data in file system for iOS

Being my first time archiving data to the file system, I would like to have my code reviewed. This simple test project works as expected, but I want to see if what I am doing is following best practices. You open the app, put in text for the textfields, press button and strings are saved and displayed in labels. When app reopens, the saved strings appear in labels.

Cars.h

@interface Car : NSObject <NSCoding>
@property (nonatomic, copy) NSString *year;
@property (nonatomic, copy) NSString *make;
@property (nonatomic, copy) NSString *model;
@end


Cars.m

NSString *const kYearKey = @"YearKey";
NSString *const kMakeKey = @"MakeKey";
NSString *const kModelKey = @"ModelKey";

@implementation Car

-(void)encodeWithCoder:(NSCoder *)aCoder {
[aCoder encodeObject:self.year forKey:kYearKey];
[aCoder encodeObject:self.make forKey:kMakeKey];
[aCoder encodeObject:self.model forKey:kModelKey];
}
self = [super init];
if (self != nil) {
}
return  self;
}
@end


ViewController.m

@interface ViewController () <UITextFieldDelegate>

@property (weak, nonatomic) IBOutlet UITextField *yearTextField;
@property (weak, nonatomic) IBOutlet UITextField *makeTextField;
@property (weak, nonatomic) IBOutlet UITextField *modelTextField;
@property (weak, nonatomic) IBOutlet UILabel *yearLabel;
@property (weak, nonatomic) IBOutlet UILabel *makeLabel;
@property (weak, nonatomic) IBOutlet UILabel *modelLabel;
@property (strong, nonatomic) NSString *yearString;
@property (strong, nonatomic) NSString *makeString;
@property (strong, nonatomic) NSString *modelString;

- (IBAction)doneButton:(id)sender;

@end

@implementation ViewController

}

NSString *path = [self archivePath];
Car *returnedCarInfo = [NSKeyedUnarchiver unarchiveObjectWithFile:path];

NSString *year = [[NSString alloc] initWithFormat:@"%@", returnedCarInfo.year];
NSString *make = [[NSString alloc] initWithFormat:@"%@", returnedCarInfo.make];
NSString *model = [[NSString alloc] initWithFormat:@"%@", returnedCarInfo.model];

_yearLabel.text = year;
_makeLabel.text = make;
_modelLabel.text = model;
}

-(BOOL)saveCarInfo {

NSString *path = [self archivePath];

Car *myCar = [[Car alloc] init];
myCar.year = _yearString;
myCar.make = _makeString;
myCar.model = _modelString;

//archive the object to the file & return YES on success
return [NSKeyedArchiver archiveRootObject:myCar toFile:path];

}

- (IBAction)doneButton:(id)sender {

_yearString = [[NSString alloc] initWithString:_yearTextField.text];
_makeString = [[NSString alloc] initWithString:_makeTextField.text];
_modelString = [[NSString alloc] initWithString:_modelTextField.text];

BOOL success = [self saveCarInfo];
if (success) {
}
}

-(NSString *)archivePath {

NSArray *documentDirectories= NSSearchPathForDirectoriesInDomains(NSDocumentDirectory,
//get the document directory from that list
NSString *documentDirectory = [documentDirectories firstObject];

return [documentDirectory stringByAppendingPathComponent:@"savedText.txt"];
}

-(BOOL)textFieldShouldReturn:(UITextField *)textField {

[textField resignFirstResponder];

return YES;
}

@end


The code looks ok. I would just comment on two things:

• Why do you keep individual fields as properties in your view controller when you can just retain a reference to a Car object?

• You don't need to connect outlets to all labels unless you're planning to change them in the code (localize or customize them for example)

• Thanks. To answer your question: pure and simple inexperience. I changed the saveCarInfo method to myCar.year = [[NSString alloc] initWithString:_yearTextField.text]; This make mush more sense. The reason I used copy is because this is how iOS 7 Programming Cookbook explained in the chapter 'Saving Objects to files'.
– KFP
Dec 11 '14 at 16:15
• From Big Nerd Ranch book iOS Programming: "When a property points to an instance of a class that has a mutable subclass(NSString/NSMutableString), you should set its memory management attribute to copy
– KFP
Dec 11 '14 at 16:29
• I guess that would make sense. Let me correct my answer. Actually... found another point :)
– sha
Dec 11 '14 at 16:38
• Concerning the label outlets:I normally wouldn't have done that, but for the sake of testing out the archiving I used labels. I could have just used NSLog()
– KFP
Dec 11 '14 at 16:53

Overall it looks pretty good. Some minor points:

if (self != nil) {


This can just be if (self). This is shorter and the style is better.

BOOL success = [self saveCarInfo];
if (success) {


This can just be if ([self saveCarInfo]) for the same reason as above.

return [documentDirectory stringByAppendingPathComponent:@"savedText.txt"];


It's not very extensible to have the save path hard coded like the above. It's fine while the scope of the program is small, but if it gets much larger, this will become a problem. Also, the method name [self archivePath] seems a little funny, since it is returning a file and not a path.

I'm not sure, but I think that the following will crash if the path is invalid or if the object ends up being nil:

Car *returnedCarInfo = [NSKeyedUnarchiver unarchiveObjectWithFile:path];

NSString *year = [[NSString alloc] initWithFormat:@"%@", returnedCarInfo.year];
NSString *make = [[NSString alloc] initWithFormat:@"%@", returnedCarInfo.make];
NSString *model = [[NSString alloc] initWithFormat:@"%@", returnedCarInfo.model];


It is worthwhile to have a check to make sure that returnedCarInfo is not nil before trying to use values from it.

Related to this, there is no fail safe or default information for the Car object if the values of the strings are not able to be set after loading. I think it would be better to have some default values so that the labels will display something even if the Car values are nil. I see that you have a check that [self saveCarInfo] succeeded before loading with [self loadCarInfo] in one place, but you also do [self loadCarInfo] inside viewDidLoad without performing any check.

• Great points! I'm now using a string property ( @property (nonatomic, strong) NSString *fileName;) and setting it _fileName = @"savedText.txt"; and then ..stringByAppendingPathComponent:_fileName]; I'm assuming this is what you meant. In viewDidLoad I don't run saveCarInfo because I'm not saving anything, just loading the values. I am using if (returnedCarInfo) in laodCarInfo, so this checks first for values,
– KFP
Dec 12 '14 at 18:29