Get string without deserialization #88
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chrissainty
requested changes
Jun 26, 2020
| @@ -248,6 +266,38 @@ public T GetItemInternal<T>(string key) | |||
| } | |||
| } | |||
|
|
|||
| public object GetItemInternal(string key) | |||
There was a problem hiding this comment.
Suggested change
| public object GetItemInternal(string key) | |
| private object GetItemInternal(string key) |
Is there a reason for this to be public? I'm guessing you've gone with this as the method this replaces was public but that shouldn't have been
There was a problem hiding this comment.
Also, do we need to keep the old GetItemInternal method?
There was a problem hiding this comment.
I don't know, I don't remember to have touched access level.
But it is used by RaiseOnChangingSync (that should be renamed RaiseOnChanging).
Co-authored-by: Chris Sainty <chrissainty@users.noreply.github.com>
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implemented a GetString method, with directly returns the string saved in local storage without any attempt to parse it.
This can lead to errors in case the string is a custom serialization (like from using Newtonsoft, see #18 ) or a string which simply starts/ends with double quotes or curly braces but it's not a valid Json.
This issue is also demonstrated by the sample app: saving valid json won't appear below "Value Read from local storage" which yield to a blank resul, as the API tries to parse the string.
"String Read from local storage", added in this PR, correctly displays the json string.
Added a new non-generic LocalStorageService.GetItemInternal, and replaced its use inside RaiseOnChangingSync method which directly returns an object, if deserialization fails, it catches the JsonException and returns the string as stored.