HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Saving Separate User Profiles

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
userprofilessavingseparate

Problem

I am modularizing some of my code in order to make it reusable in future projects. The latest thing that I have created is a simple class that saves user profiles. The list of profiles is loaded into a view, and the user can add a new profile to the list and select which profile is currently active. When a profile is added to the list a directory with a matching name is created. All files unique to that user will be saved in their directory.

In other classes, objects will check if there is a current active profile (by looking for the existence of a certain file). If it exists, the objects will save and load using the active profile directory.

I'm looking for any kind of feedback on this code, but specifically, am I using class methods in an appropriate way? Is the interface easy to understand? I'm not planning to make a public library, but I would still like to do things in the best way possible.

MPProfileTools.h

#import 

@interface MPProfileTools : NSObject

//profiles are stored as an array of strings
+(NSMutableArray *) allProfiles;
+(void) saveProfileList:(NSMutableArray *)profileList;

//name of the current active profile
+(NSString *) activeProfile;

//create the directory and the active profile file
+(void) setActiveProfile:(NSString *)profile;

//deletes the associated directory
//also deletes the active profile file if it is currently selected
+(void) deleteProfile:(NSString *)profile;

@end


MPProfileTools.m

```
#import "MPPathTools.h"
#import "MPProfileTools.h"

@implementation MPProfileTools

+(NSMutableArray *) allProfiles {
NSString *path = [MPPathTools allProfilesPath];
NSMutableArray *profilesToLoad = [NSKeyedUnarchiver unarchiveObjectWithFile:path];
return profilesToLoad;
}
+(void) saveProfileList:(NSMutableArray *)profileList {
NSString *path = [MPPathTools allProfilesPath];
[NSKeyedArchiver archiveRootObject:profileList toFile:path];
}

+(NSString *) activeProfile {
return [NSKeyedUnarchiver unarchiveObjec

Solution

It's a good start. Somewhat difficult to understand logic behind the scene without seeing your code for MPPathTool, but I'd like to start commenting a bit on MPProfileTools

-
Name MPProfileTools doesn't mean much. Naming classes and methods are important. What is Tools? Why don't call this class MPUserProfiles?

-
Looks like you want to use singleton pattern here, so use it properly. Add a method to return sharedInstance of MPProfileTools class and make all access methods not static.

-
You don't need to return and assign mutable arrays for list of profiles. I would even consider changing it to be just a property that you can create custom setter and getter for.

-
Naming convention. Don't put List inside name of something that is not list.

So your class interface would like this (I will leave implementation details to you):

@interface MPUserProfiles : NSObject

+ (instancetype)sharedInstance; 

// profiles are stored as an array of strings
@property (nonatomic, strong) NSArray *profiles;

// currently active profile
@property (nonatomic, strong) NSString *activeProfile;

// deletes the associated directory
// also deletes the active profile file if it is currently selected
- (void)deleteProfileDirectory:(NSString *)profile;

@end


As for the MPProfileTableViewController I have one comment: if your application is a bit more complicated then just one screen and one view controller and you use UIAlertView in other place as well I highly recommend to wrap UIAlertView/UIAlertController into a special class that would handle confirmation messages independently of OS version (there are different approaches for iOS7 and iOS8 for example) and device (again - there is difference between iPhone and iPad implementation) and provide application business logic with nice completion block interface.

This was you will not have to implement delegate for UIAlertView in each view controller in your application, and in this particular one you will not need to save copy of tableView into temp variable.

Code Snippets

@interface MPUserProfiles : NSObject

+ (instancetype)sharedInstance; 

// profiles are stored as an array of strings
@property (nonatomic, strong) NSArray *profiles;

// currently active profile
@property (nonatomic, strong) NSString *activeProfile;

// deletes the associated directory
// also deletes the active profile file if it is currently selected
- (void)deleteProfileDirectory:(NSString *)profile;

@end

Context

StackExchange Code Review Q#73736, answer score: 2

Revisions (0)

No revisions yet.