Skip to content

Adding documentation and tests that maxBy/minBy return the first max/min element (Array, List, Seq) #19140

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.

Sign up for GitHub

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

Open
ursenzler wants to merge 2 commits into
base: main
Choose a base branch
from
+35 −5
Open

Adding documentation and tests that maxBy/minBy return the first max/min element (Array, List, Seq) #19140

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1ca967e
Added description that first found max/min is returned by the maxBy a…
ursenzler Dec 5, 2025
099a0bb
added tests from maxBy and minBy that they return the first max/min e…
ursenzler Dec 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/FSharp.Core/array.fsi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1789,7 +1789,7 @@ module Array =

/// <summary>Returns the greatest of all elements of the array, compared via Operators.max on the function result.</summary>
///
/// <remarks>Throws ArgumentException for empty arrays.</remarks>
/// <remarks>Returns the first maximal element of the array if there are multiple equal maximum elements.</remarks>
///
/// <param name="projection">The function to transform the elements into a type supporting comparison.</param>
/// <param name="array">The input array.</param>
Expand Down Expand Up @@ -1852,8 +1852,8 @@ module Array =

/// <summary>Returns the lowest of all elements of the array, compared via Operators.min on the function result.</summary>
///
/// <remarks>Throws ArgumentException for empty arrays.</remarks>
///
/// <remarks>Returns the first minimal element of the array if there are multiple equal minimal elements.</remarks>
///
/// <param name="projection">The function to transform the elements into a type supporting comparison.</param>
/// <param name="array">The input array.</param>
///
Expand Down
6 changes: 4 additions & 2 deletions src/FSharp.Core/list.fsi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1479,7 +1479,8 @@ module List =

/// <summary>Returns the greatest of all elements of the list, compared via Operators.max on the function result.</summary>
///
/// <remarks>Raises <see cref="T:System.ArgumentException"/> if <c>list</c> is empty.</remarks>
/// <remarks>Returns the first maximal element of the list if there are multiple equal maximum elements.</remarks>
Copy link
Member

@T-Gro T-Gro Dec 10, 2025

Choose a reason for hiding this comment

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

But we should also keep the remark about empty input (applies also to array)

Copy link
Member

@Happypig375 Happypig375 Dec 11, 2025

Choose a reason for hiding this comment

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

Shouldn't exception documentation use <exception> instead of <remarks>?

Copy link
Member

@T-Gro T-Gro Dec 11, 2025

Choose a reason for hiding this comment

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

Yes, even better.

///
/// <param name="projection">The function to transform the list elements into the type to be compared.</param>
/// <param name="list">The input list.</param>
///
Expand Down Expand Up @@ -1538,7 +1539,8 @@ module List =

/// <summary>Returns the lowest of all elements of the list, compared via Operators.min on the function result</summary>
///
/// <remarks>Raises <see cref="T:System.ArgumentException"/> if <c>list</c> is empty.</remarks>
/// <remarks>Returns the first minimal element of the list if there are multiple equal minimal elements.</remarks>
///
/// <param name="projection">The function to transform list elements into the type to be compared.</param>
/// <param name="list">The input list.</param>
///
Expand Down
4 changes: 4 additions & 0 deletions src/FSharp.Core/seq.fsi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1716,6 +1716,8 @@ module Seq =

/// <summary>Returns the greatest of all elements of the sequence, compared via Operators.max on the function result.</summary>
///
/// <remarks>Returns the first maximal element of the sequence if there are multiple equal maximum elements.</remarks>
///
/// <param name="projection">A function to transform items from the input sequence into comparable keys.</param>
/// <param name="source">The input sequence.</param>
///
Expand Down Expand Up @@ -1780,6 +1782,8 @@ module Seq =
///
/// <returns>The smallest element of the sequence.</returns>
///
/// <remarks>Returns the first minimal element of the sequence if there are multiple equal minimal elements.</remarks>
///
/// <exception cref="T:System.ArgumentNullException">Thrown when the input sequence is null.</exception>
/// <exception cref="T:System.ArgumentException">Thrown when the input sequence is empty.</exception>
///
Expand Down
8 changes: 8 additions & 0 deletions tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Collections/ArrayModule2.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,10 @@ type ArrayModule2() =
// len = 0
CheckThrowsArgumentException(fun() -> Array.maxBy funcInt (Array.empty<int>) |> ignore)

// returns first maximal element
let max = Array.maxBy fst [|1, "a"; 2, "b"; 3, "c"; 2, "d"; 3, "e"; 1, "f" |]
if snd max <> "c" then Assert.Fail()

()

[<Fact>]
Expand Down Expand Up @@ -348,6 +352,10 @@ type ArrayModule2() =
// len = 0
CheckThrowsArgumentException(fun () -> Array.minBy funcInt (Array.empty<int>) |> ignore)

// returns first minimal element
let max = Array.minBy fst [|3, "a"; 2, "b"; 1, "c"; 2, "d"; 1, "e"; 3, "f" |]
if snd max <> "c" then Assert.Fail()

()


Expand Down
8 changes: 8 additions & 0 deletions tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Collections/ListModule2.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,10 @@ type ListModule02() =
// empty List
CheckThrowsArgumentException ( fun() -> List.maxBy (fun () -> 1) List.empty )

// returns first maximal element
let max = List.maxBy fst [1, "a"; 2, "b"; 3, "c"; 2, "d"; 3, "e"; 1, "f" ]
if snd max <> "c" then Assert.Fail()

()

[<Fact>]
Expand Down Expand Up @@ -324,6 +328,10 @@ type ListModule02() =
let funcEpt () = 1
CheckThrowsArgumentException ( fun() -> List.minBy funcEpt List.empty)

// returns first minimal element
let max = List.minBy fst [ 3, "a"; 2, "b"; 1, "c"; 2, "d"; 1, "e"; 3, "f" ]
if snd max <> "c" then Assert.Fail()

()

[<Fact>]
Expand Down
8 changes: 8 additions & 0 deletions tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Collections/SeqModule2.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -969,6 +969,10 @@ type SeqModule2() =
let nullSeq:seq<'a> = null
CheckThrowsArgumentNullException (fun () ->Seq.maxBy funcInt nullSeq |> ignore)

// returns first maximal element
let max = Seq.maxBy fst (seq { 1, "a"; 2, "b"; 3, "c"; 2, "d"; 3, "e"; 1, "f" })
if snd max <> "c" then Assert.Fail()

()

[<Fact>]
Expand All @@ -991,6 +995,10 @@ type SeqModule2() =
let nullSeq:seq<'a> = null
CheckThrowsArgumentNullException (fun () ->Seq.minBy funcInt nullSeq |> ignore)

// returns first minimal element
let max = Seq.minBy fst (seq { 3, "a"; 2, "b"; 1, "c"; 2, "d"; 1, "e"; 3, "f" })
if snd max <> "c" then Assert.Fail()

()


Expand Down
Loading