-
Notifications
You must be signed in to change notification settings - Fork 54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove IImageCache
#2391
Remove IImageCache
#2391
Conversation
{ | ||
try | ||
{ | ||
if (optionImage.TryPickT0(out var uri, out var imageStoredFile)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is technically not incorrect, but it 'feels' a bit weird. Technically you should prefer loading a local file, so you really should be asking 'do I have a local file' instead of 'do I have a URL'.
While in this case there isn't a functional difference, because it's a OneOf; I thought I'd point it out. Someone looking at this code may misinterpret it; since the Try
methods in a branch like this are usually used with dictionaries. I had to double take, until I saw it was a OneOf
Consider flipping the condition, i.e. TryPickT1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've simplified the code using Match
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rest is ok, just the comment above.
No description provided.