Skip to content

Conversation

@gdziadkiewicz
Copy link
Contributor

@gdziadkiewicz gdziadkiewicz commented Oct 21, 2018

@msftclas
Copy link

msftclas commented Oct 21, 2018

CLA assistant check
All CLA requirements met.

@gdziadkiewicz gdziadkiewicz force-pushed the feature/AddTryExactlyOne branch from 1281822 to cbfba66 Compare October 21, 2018 19:23
@gdziadkiewicz gdziadkiewicz changed the title [WIP] Add TryExactlyOne for array, list and seq. Add TryExactlyOne for array, list and seq. Oct 21, 2018
@0x53A
Copy link
Contributor

0x53A commented Oct 21, 2018

I don't have too strong an opinion about this, but just wanted to note that the corresponding LINQ function differs:

[ "1" ].SingleOrDefault() => "1"
[  ].SingleOrDefault() => null
[ "1", "2" ].SingleOrDefault() => Exception

@abelbraaksma
Copy link
Contributor

@0x53A, I think the 'SingleXXX' LINQ methods are semantically similar but do not need to be exactly the same as the 'TryXXX' functions. The latter should never throw (except perhaps if the argument is null), in line with existing Try functions. I'd favor the current implementation over throwing an exception in 'TryXXX' functions.

@gdziadkiewicz
Copy link
Contributor Author

I also expect from tryXXX to not throw. Lack of null check is deliberate - all of the List module functions assume that provided list is not null.

@reed-adams
Copy link

I may be the oddball expecting it to throw on 2+ elements.

The reason is because myself and others (based on the work I see from people) intuitively believe the try* functions are safe alternatives to the null-producing functions given by the *OrDefault Linq variations, which throws in the case of SingleOrDefault. In my experience "try" is an exact mental match for "instead of null/magic number". I can only believe that diverging from the "obvious" behavior will place another straw on the least-surprises/why-can't-I camel that I (and again, others) have to contend with in the workplace while pushing (uphill) for the adoption of F# (plug plug fsharp/fslang-suggestions#103). It isn't the end of the world, but things like this are felt by people in places that are less than excited about "new technology."

Anecdotally, we use SingleOrDefault to express "exactly one item or null if empty" - allowing it to be something of an early warning for data issues. In our work we would find it extremely surprising if functions such as these allowed "data issue" to have exactly the same non-exception representation as "no item."

Again, while not required, I suspect parity to the Linq implementations and expectations would go a very long way in reducing surprises and allowing a more seamless transition from other .NET languages.

First -> find
FirstOrDefault -> tryFind
Single -> exactlyOne
SingleOrDefault -> tryExactlyOne

Anyway, these are just my experiences and observations. Thank you for considering them.

@gdziadkiewicz
Copy link
Contributor Author

Wouldn't it be inconsistent for a tryXXX function to throw? I will check how many of the tryXXX functions in FSharp.Core throw and come back with this info.

I believe that most of the people (me and my team for sure) use tryXXX functions to avoid checks on their side and catching exceptions for expected situations (as those checks/catches don't compose well). I suspect that making tryXXX throw would be more surprising for current F# users than making it return None for newcomers from other .NET languages.

It would be great to hear more opinions on this.

@0x53A
Copy link
Contributor

0x53A commented Oct 25, 2018

I don't think tryXXX should throw - but I'd like to also have a throwing version. In the RFC, one mention was zeroOrOne, which kinda fits.

@cartermp
Copy link
Contributor

These functions should not throw given the "try" prefix. In F# the convention is that tryXXX returns an optional (as per the implementation).

This will also require an RFC. @gdziadkiewicz Can you submit one please? https://github.com/fsharp/fslang-design

@gdziadkiewicz
Copy link
Contributor Author

@cartermp Yes, I will submit it tomorrow.

@reed-adams
Copy link

@cartermp Understood that's how it will be implemented.

Regarding comments in: fsharp/fslang-suggestions#137 by @0x53A and @abelbraaksma

I also have need of a SingleOrDefault-style function that throws. This zeroOrOne, what might be the signature of such a function? What does it return if its the zero case? My mind immediately jumps to tryZeroOrOne to indicate the Option result of zero or one item, but as per your comments this new function would likewise not be allowed to throw and would become a clone of this tryExactlyOne function.

What is the resolution to such things?

@cartermp
Copy link
Contributor

@reed-adams my suggestion is to comment on fsharp/fslang-suggestions#137 with suggested functions and signatures, since that's where discussion over things like this typically occurs.

@reed-adams
Copy link

@cartermp I wasn't completely certain where my comment should go as it seemed to sort of straddle concerns in two threads. Thank you for the direction.

@abelbraaksma
Copy link
Contributor

What does it return if its the zero case? My mind immediately jumps to tryZeroOrOne to indicate the Option result of zero or one item, but as per your comments this new function would likewise not be allowed to throw and would become a clone of this tryExactlyOne function.

@reed-adams, the short answer, taken from other languages and that I think fits: zeroOrOne should return either an empty sequence or a sequence-of-one. Otherwise it should throw. The tryZeroOrOne should return Some/None of the same.

But I agree, it is probably better to move that discussion to fsharp/fslang-suggestions#137, so I'll add the more elaborate answer there ;)

Copy link
Contributor

@cartermp cartermp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @gdziadkiewicz for the RFC and this implementation. I think it looks good.

@abelbraaksma
Copy link
Contributor

abelbraaksma commented Nov 6, 2018

For reference, the RFC can be found here: FS-1066 Thanks!

Copy link
Contributor

@KevinRansom KevinRansom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this submission.

@KevinRansom KevinRansom merged commit 58a48e1 into dotnet:master Nov 12, 2018
KevinRansom pushed a commit that referenced this pull request Nov 13, 2018
* Remove dependence on runfsc.cmd and runfsc.sh (#5882)

* Remove dependence on runfsc.cmd and runfsc.sh

* White space

* Add TryExactlyOne for array, list and seq. (#5804)

* FS-1065 Value Option Parity (#5772)

* Initial FS-1065 implementation

* Undo removal of compilationrepresentation suffix and update surface area

* Whoopise, add the suffix to the impl file

* Update coreclr surface area

* Revert the FSComp changes that somehow got picked up

* newline

* Consume internal VOption module functions

* More internal voption module functions
@gdziadkiewicz gdziadkiewicz deleted the feature/AddTryExactlyOne branch November 15, 2018 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants