Set initial cache expiry time after all settings have been loaded

[Avani Gupta (avanigupta)]

Fix #172

[Jimmy Campbell (jimmyca15)]

Looks good. One question. What's the purpose of the last commit to initialize cachedData to string.Empty?

[Avani Gupta (avanigupta)]

Looks good. One question. What's the purpose of the last commit to initialize cachedData to string.Empty?

As Abhilash suggested in this comment: #204 (comment)

I moved the cache exporting logic back to the end of LoadAll. So before exporting, we just need to check if the data was loaded from server or from offline cache. If it was loaded from offline cache, we don't need to export the same data again. That's why I initialized cachedData at the beginning of LoadAll so that it can be accessed both inside and outside the catch block.

[Jimmy Campbell (jimmyca15)]

Looks good. One question. What's the purpose of the last commit to initialize cachedData to string.Empty?

As Abhilash suggested in this comment: #204 (comment)

I moved the cache exporting logic back to the end of LoadAll. So before exporting, we just need to check if the data was loaded from server or from offline cache. If it was loaded from offline cache, we don't need to export the same data again. That's why I initialized cachedData at the beginning of LoadAll so that it can be accessed both inside and outside the catch block.

I get that. Not sure why it is initialized to string.Empty instead of null. The default initialization of a string is null so the fact that it's explicitly initialized to an empty string makes it look like it's for some purpose.

[Avani Gupta (avanigupta)]

Looks good. One question. What's the purpose of the last commit to initialize cachedData to string.Empty?

As Abhilash suggested in this comment: #204 (comment)
I moved the cache exporting logic back to the end of LoadAll. So before exporting, we just need to check if the data was loaded from server or from offline cache. If it was loaded from offline cache, we don't need to export the same data again. That's why I initialized cachedData at the beginning of LoadAll so that it can be accessed both inside and outside the catch block.

I get that. Not sure why it is initialized to string.Empty instead of null. The default initialization of a string is null so the fact that it's explicitly initialized to an empty string makes it look like it's for some purpose.

Good point, thanks! Reverted the last commit. We also don't need to check if OfflineCache.Import returned empty string because it always returns null if the encrypted data in offline cache file is invalid (or throws an exception if the file cannot be accessed). So a simple null check will suffice.