refactor(init): build invalid instance if datafile cannot be parsed. #209
Conversation
mikeproeng37
requested review from
aliabbasrizvi,
shihpatrick and
thomaszurkan-optimizely
| @VisibleForTesting | ||
| void initialize() { | ||
| void initialize(@Nonnull String datafile, @Nullable ProjectConfig projectConfig) { | ||
| if (projectConfig == null) { |
There was a problem hiding this comment.
This step de-couples the "building" of the Optimizely instance from the "initialization", which is where we start populating state.
There was a problem hiding this comment.
I think that we should probably discuss this. As far as I know for threading purposes, you would not reuse these components but instead, initialize a new instance. This actually brings up an interesting point of Android datafile handler: Should the datafile handler reinitialize or have a shutdown mechanism?
There was a problem hiding this comment.
Yep, will take this offline.
Pull Request Test Coverage Report for Build 602
💛 - Coveralls |
| @VisibleForTesting | ||
| void initialize() { | ||
| void initialize(@Nonnull String datafile, @Nullable ProjectConfig projectConfig) { | ||
| if (projectConfig == null) { |
There was a problem hiding this comment.
I think that we should probably discuss this. As far as I know for threading purposes, you would not reuse these components but instead, initialize a new instance. This actually brings up an interesting point of Android datafile handler: Should the datafile handler reinitialize or have a shutdown mechanism?
Summary
Optimizelyinstance with theisValidproperty set tofalse. When accessing any of the SDK APIs, we will first check if the instance is valid before proceeding. If not valid, we will return the sensible defaults for each API.ErrorHandlerso the developer can be notified of these kinds of failures.This does constitute a breaking API change since we will no longer be throwing
ConfigParseExceptionfrom theBuilder.build()method. As such, this will be released in version 3 of the SDK.Missing
This PR is to get initial feedback and is therefore missing unit tests for the invalid state in feature variable APIs. Will add those once I get some feedback regarding this approach.