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

Add Option.ofDefault #630

Open
cmeeren opened this Issue Dec 6, 2017 · 12 comments

Comments

Projects
None yet
7 participants
@cmeeren

cmeeren commented Dec 6, 2017

I propose we add a function to the Option module that returns None if the input value is the default of its type:

/// Returns None if the input is equal to the default uninitialized value
/// of its type.
let ofDefault value =
  if value = Unchecked.defaultof<_> then None else Some value

This is useful in contexts where you call non-F# code (or non-idiomatic F# code) and need to convert values like null, DateTime.MinValue, etc. to an Option.

The existing way of approaching this problem in F# is to simply write

if value = Unchecked.defaultof<_> then None else Some value

wherever you need it, or alternatively more specific stuff like

if value = DateTime.MinValue then None else Some value

Note: This could also be combined with implementing a corresponding toDefault function, which could be useful in the same circumstances but would of course be considered generally unsafe.

Pros and Cons

The advantages of making this adjustment to F# are simplicity in using the option type when interfacing with non-F# (and non-idiomatic F#) codebases.

The disadvantages of making this adjustment to F# are, as far as I can see, none, though there may be some caveats with Unchecked.defaultOf that I don't know about.

Extra information

Estimated cost (XS, S, M, L, XL, XXL): XS

Affidavit (please submit!)

Please tick this by placing a cross in the box:

  • This is not a question (e.g. like one you might ask on stackoverflow) and I have searched stackoverflow for discussions of this issue
  • I have searched both open and closed suggestions on this site and believe this is not a duplicate
  • This is not something which has obviously "already been decided" in previous versions of F#. If you're questioning a fundamental design decision that has obviously already been taken (e.g. "Make F# untyped") then please don't submit it.

Please tick all that apply:

  • This is not a breaking change to the F# language design
  • I or my company would be willing to help implement and/or test this
@dsyme

This comment has been minimized.

Show comment
Hide comment
@dsyme

dsyme Dec 8, 2017

Collaborator

I'm fairly happy with this suggestion. But should it have a companion Option.toDefault : 'a option -> 'a?

let toDefault xopt = match xopt with None -> Unchecked.defaultof<_> | Some x -> x

That would however be an "unchecked" operation that is a source of default values (like Array.zeroCreate and Unchecked.defaultof<_>)

Collaborator

dsyme commented Dec 8, 2017

I'm fairly happy with this suggestion. But should it have a companion Option.toDefault : 'a option -> 'a?

let toDefault xopt = match xopt with None -> Unchecked.defaultof<_> | Some x -> x

That would however be an "unchecked" operation that is a source of default values (like Array.zeroCreate and Unchecked.defaultof<_>)

@cmeeren

This comment has been minimized.

Show comment
Hide comment
@cmeeren

cmeeren Dec 8, 2017

Was that a suggestion or a question? Because I already mentioned it in the proposal.

cmeeren commented Dec 8, 2017

Was that a suggestion or a question? Because I already mentioned it in the proposal.

@dsyme

This comment has been minimized.

Show comment
Hide comment
@dsyme

dsyme Dec 8, 2017

Collaborator

Because I already mentioned it in the proposal.

@cmeeren Ah yes, I didn't see it! my apologies

Collaborator

dsyme commented Dec 8, 2017

Because I already mentioned it in the proposal.

@cmeeren Ah yes, I didn't see it! my apologies

@cmeeren

This comment has been minimized.

Show comment
Hide comment
@cmeeren

cmeeren Dec 8, 2017

I'm in favor of including toDefault. I could see both of these being useful when interfacing with non-F#(-idiomatic) code, lowering the bar for actually using option types in such contexts (which is a good thing).

cmeeren commented Dec 8, 2017

I'm in favor of including toDefault. I could see both of these being useful when interfacing with non-F#(-idiomatic) code, lowering the bar for actually using option types in such contexts (which is a good thing).

@realvictorprm

This comment has been minimized.

Show comment
Hide comment
@realvictorprm

realvictorprm Dec 8, 2017

Member
Member

realvictorprm commented Dec 8, 2017

@vasily-kirichenko

This comment has been minimized.

Show comment
Hide comment
@vasily-kirichenko

vasily-kirichenko Dec 9, 2017

I don't like it because the functions do the same as toObj and ofObj for nullable types.

vasily-kirichenko commented Dec 9, 2017

I don't like it because the functions do the same as toObj and ofObj for nullable types.

@cmeeren

This comment has been minimized.

Show comment
Hide comment
@cmeeren

cmeeren Dec 9, 2017

Since ofDefault and toDefault are more general, perhaps they can deprecate toObj and ofObj?

Or perhaps toObj and ofObj could serve as aliases for ofDefault and toDefault? I'm guessing this would not be a breaking change, though there might be some reflection stuff I don't know about that depends on the null type constraint of toObj and ofObj.

They could of course also live side by side. Anyway, currently there's no built-in way to convert non-nullable types from/to option like I described.

cmeeren commented Dec 9, 2017

Since ofDefault and toDefault are more general, perhaps they can deprecate toObj and ofObj?

Or perhaps toObj and ofObj could serve as aliases for ofDefault and toDefault? I'm guessing this would not be a breaking change, though there might be some reflection stuff I don't know about that depends on the null type constraint of toObj and ofObj.

They could of course also live side by side. Anyway, currently there's no built-in way to convert non-nullable types from/to option like I described.

@enricosada

This comment has been minimized.

Show comment
Hide comment
@enricosada

enricosada Jan 8, 2018

Since ofDefault and toDefault are more general, perhaps they can deprecate toObj and ofObj?

they cannot. a DateTime is not the same as a Nullable<DateTime>, and the minvalue is a valid value too.

why 0 |> Option.ofDefault should be None? 0 is a valid int.

the only strange case afaik is DateTime.

Adding an additional function with a non obvious meaning make things more complicated, not less.

If DateTime is the issue (probably is the only one), is possibile to add an extension method to DateTime for that.

enricosada commented Jan 8, 2018

Since ofDefault and toDefault are more general, perhaps they can deprecate toObj and ofObj?

they cannot. a DateTime is not the same as a Nullable<DateTime>, and the minvalue is a valid value too.

why 0 |> Option.ofDefault should be None? 0 is a valid int.

the only strange case afaik is DateTime.

Adding an additional function with a non obvious meaning make things more complicated, not less.

If DateTime is the issue (probably is the only one), is possibile to add an extension method to DateTime for that.

@Rickasaurus

This comment has been minimized.

Show comment
Hide comment
@Rickasaurus

Rickasaurus Jan 9, 2018

Sorry, got it backward, please ignore me.

Rickasaurus commented Jan 9, 2018

Sorry, got it backward, please ignore me.

@enricosada

This comment has been minimized.

Show comment
Hide comment
@enricosada

enricosada Jan 9, 2018

@cmeeren what is an example of another type who need this?

  • DateTime because MinValue.

  • reference types and nullable already work with null using Option.ofObj

  • ... ? other examples? what is your interop scenario?

enricosada commented Jan 9, 2018

@cmeeren what is an example of another type who need this?

  • DateTime because MinValue.

  • reference types and nullable already work with null using Option.ofObj

  • ... ? other examples? what is your interop scenario?

@cmeeren

This comment has been minimized.

Show comment
Hide comment
@cmeeren

cmeeren Jan 9, 2018

@enricosada I agree it doesn't necessarily make much sense for primitive values like int, and DateTime and DateTimeOffset are the only ones I've needed this for so far, but it's relevant for all struct types since they can't be null.

cmeeren commented Jan 9, 2018

@enricosada I agree it doesn't necessarily make much sense for primitive values like int, and DateTime and DateTimeOffset are the only ones I've needed this for so far, but it's relevant for all struct types since they can't be null.

@voronoipotato

This comment has been minimized.

Show comment
Hide comment
@voronoipotato

voronoipotato Jan 10, 2018

Old vb code frequently has default value to mean null, yes its bad and I hate it but this might be helpful for people making the leap from VB.Net to F#.

voronoipotato commented Jan 10, 2018

Old vb code frequently has default value to mean null, yes its bad and I hate it but this might be helpful for people making the leap from VB.Net to F#.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment