-
Notifications
You must be signed in to change notification settings - Fork 5
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
Optional strings #2
Optional strings #2
Conversation
self.description = dict["description"] as? String | ||
|
||
let overrides = try? dict["overrides"] | ||
.flatMap { $0 as? [String: Any] }? |
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.
Should this be compactMap
?
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.
In this case we're actually flatMap
'ing the Optional
rather than the Collection
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.
Fair enough
if let val = value as? T { | ||
return val | ||
} | ||
if let nilLiteralType = T.self as? ExpressibleByNilLiteral.Type { |
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.
Can you add some doc comments on this. I've read it about 10 times and think I'm understnading it now, but doesn't hurt to add something in to explain what is a pretty opaque piece of code without significant context.
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.
In the end, I'm assuming it's entirely because you can't convert NSNull
to nil
?
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.
Yeah, this was fun...
This is to handle the case where T: Optional
. If the typecast to T
fails (in this case NSNull.Type
-> Optional<String>
) we want to return nil
, but in a way that isn't a failure.
nil
can't be assigned to defaultValue
for example, because as far as ConfigurationProperty
knows it's not optional. We also can't construct the Optional
directly, because we don't know the Wrapped
type.
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.
Hopefully a bit clearer with that documentation.
Adds the definition of
String?
asoptionalString
toPropertyType
, so we can construct optional strings as required.Most of the heavy lifting happens in
ConfigurationProperty
, where previously we'd bail if converting the default value / overrides toT
failed.To detect whether conversion to
T
being nil isn't a failure, we check to see whetherT: ExpressibleByNilLiteral
(Optional is the only thing that conforms to this). If it is, we have to do a bit of a dance to doT -> ExpressibleByNilLiteral -> T
so we can successfully initialise. The rest of it is relatively self-explanatory.