chore (OPTDataStore): Remove print statements and change memory store default backing store. #310
Conversation
thomaszurkan-optimizely
changed the title
|
|
||
|
|
| set { | ||
|
|
||
| } |
| func testPerformanceExample() { | ||
| // This is an example of a performance test case. | ||
| self.measure { |
| // This is an example of a functional test case. | ||
| // Use XCTAssert and related functions to verify your tests produce the correct results. |
|
|
||
| XCTAssert(v2 == value) |
There was a problem hiding this comment.
Let's clean up here test data which may be left from previous test:
UserDefaults.standard.removeObject(forKey: "testUserDefaultsTooBig")
There was a problem hiding this comment.
it is never saved because it is too big. :)
Co-Authored-By: Jae Kim <[email protected]>
| public func removeItem(sdkKey:String) { | ||
| if let store = self as? DataStoreUserDefaults { | ||
| store.removeItem(sdkKey: sdkKey) | ||
| } | ||
| else if let store = self as? DataStoreFile<Data> { | ||
| store.removeItem(sdkKey: sdkKey) | ||
| } | ||
| else if let store = self as? DataStoreMemory<Data> { | ||
| store.removeItem(sdkKey: sdkKey) |
There was a problem hiding this comment.
It must be for backward compatibility. Can't we leave it empty? All our datastores override it. Also we can move it to the end of OPTDataStore for readability.
public func removeItem(sdkKey: String) { }
There was a problem hiding this comment.
no because that is the method called on the extension (i.e. the other implementation is not an override since it is an extension). the only alternative is to add it to the data store protocol directly.
| } | ||
| else { | ||
| #if os(tvOS) | ||
| let store = DataStoreUserDefaults() |
There was a problem hiding this comment.
It looks good, but I'm concerned about the datafile size which can grow big more than MB. They may not fit into UserDefaults max configs in many cases. Also with event queues, it can take substantial spaces in UserDefaults even if they can fit in 128KB each.
There was a problem hiding this comment.
even if you reach 512k in tvOS, the action is that the user is notified, it can actually grow beyond that. So, this is probably pretty safe. That said, if the datafile is too big, what choice do they have on tvOS?
There was a problem hiding this comment.
basically, the same thing will happen as happens now on tvOS. start async, caching fails, but you can continue. the bigger question may be do we want to disable polling for tvOS? i am really curious why the simulator doesn't fail.
There was a problem hiding this comment.
Polling is optional, so we don't need to block it. They won't turn it on if not working.
We can also consider "Cache" folder as done in ObjC. Both methods are not fully reliable for keeping datafile cache and events.
Summary