MapboxGL Startup Infrastructure #1329
Conversation
…pDelegate and exposing MGLMapView.initWithFrame to support it
| if (accessToken) { | ||
| // Store to preferences so that we can launch the app later on without having to specify | ||
| // token. | ||
| [[NSUserDefaults standardUserDefaults] setObject:accessToken forKey:@"access_token"]; |
There was a problem hiding this comment.
Nit: A common strategy for user defaults keys is to make them match the names of the constants that define them. So you’d define a constant called MGLMapboxAccessTokenDefaultsKey with the value MGLMapboxAccessToken in MGLTypes.h and use that constant here. Obviously it doesn’t matter as long as you’re consistent, but it’s one less thing to worry about naming.
There was a problem hiding this comment.
We'd be better off putting things in a constant to avoid future potential problems with the @"access_token" magic string, too.
|
@1ec5's feedback has been incorporated. The next step is to split MapboxGL.h back into a regular Umbrella Header file. |
…the singleton code into MGLAccountManager.h and .m
…any iOS get naming conventions
| // Can be called from any thread. Called implicitly from any | ||
| // public class convenience methods. | ||
| // | ||
| + (instancetype) sharedInstanceWithAccessToken:(NSString *)token { |
There was a problem hiding this comment.
This method’s name is misleading. It implies that there can be more than one shared instance, that we’re mapping access tokens to shared instances somehow. What happens when this class begins to manage more than just an access token? +sharedInstanceWithUserName:accessToken:expiryDate:cachingPragma:?
I still think the two options in #1329 (comment) are the best ways forward.
There was a problem hiding this comment.
If I understand correctly using the instance methods would then have the initial setup in the AppDelegate be something like:
[[MGLAcountManager sharedManager] setAccessToken:@"FOO"]
[MGLAcountManager sharedInstanceWithAccessToken:@"FOO"]
How about renaming it to managerWithInititalAccessTokenValue or a derivative?
FWIW, I'm not terribly fond of the C style option so I'd like to get the first option working.
There was a problem hiding this comment.
[[MGLAcountManager sharedManager] setAccessToken:@"FOO"]
The way we do this in MBXMapKit is more brief:
[MGLAccountManager setAccessToken:]
Which is basically just a shortcut to the above, but doesn't require the user to worry about shared instances.
There was a problem hiding this comment.
Also, +setAccessToken: could return an instancetype for easy access instead of void.
There was a problem hiding this comment.
I ended up implementing +setAccessToken that returned void instead of instancetype as the latter was causing crashes.
|
Ended up pulling the initial |
| @@ -28,6 +28,7 @@ | |||
| #import "SMCalloutView.h" | |||
|
|
|||
| #import "MGLMapboxEvents.h" | |||
| #import "MapboxGL.h" | |||
There was a problem hiding this comment.
This import statement likely makes some of the other ones redundant.
For #1225