patternMinor
Poker Hands Kata in F#
Viewed 0 times
pokerkatahands
Problem
I'm a F# newbie, and would like to share my implementation of the Poker Hands Kata problem to get some feedback.
```
module PokerHands
open System.Text.RegularExpressions
type Figure =
| Two | Three | Four | Five
| Six | Seven | Eight | Nine
| Ten | Jack | Queen | King | Ace
type Suit = Diamonds | Spades | Hearts | Clubs
type Card = Figure * Suit
type Rank =
| HighCard of Figure * Rank option
| Pair of Figure * Rank
| TwoPair of Figure Figure Rank
| ThreeOfKind of Figure
| Straight of Figure
| Flush of Rank
| FullHouse of Figure
| FourOfKind of Figure
| StraightFlush of Rank
type Player = {
Name: string;
Cards: Card list;
Hand: Rank;
}
let SortDesc sequence = sequence |> Seq.toList |> List.sortWith (fun x y -> compare y x)
let SortDescBy f sequence = sequence |> List.sortWith (fun x y -> compare (f y) (f x))
let rec BuildHighCard (figures : Figure list) =
match SortDesc figures with
| [h] -> HighCard(h, None)
| h :: t -> HighCard(h, Some(BuildHighCard(t)))
| _ -> failwith "empty"
let isFlush (cards : Card list) =
cards |> Seq.distinctBy snd |> Seq.length = 1
let isStraight (cards : Card list) =
let figs = cards |> List.map fst |> List.sort
let flag = ref true
for i in {1 .. List.length figs - 1} do
if (compare figs.[i-1] figs.[i]) <> -1 then flag.Value List.map fst))) else None
let (|IsFlush|_|) cards =
if isFlush cards then Some(Flush(BuildHighCard(cards |> List.map fst))) else None
let (|IsStraight|_|) cards =
if isStraight cards then Some(Straight(cards |> List.minBy fst |> fst)) else None
let (|IsGrouped|_|) (counts : int list) cards =
let groups = cards |> Seq.countBy fst |> Seq.toList |> SortDescBy snd
if groups |> List.map snd = counts then Some(groups |> List.map fst) else None
let DetermineRank (cards : Card list) =
match cards with
| IsStraightFlush straightFlus
```
module PokerHands
open System.Text.RegularExpressions
type Figure =
| Two | Three | Four | Five
| Six | Seven | Eight | Nine
| Ten | Jack | Queen | King | Ace
type Suit = Diamonds | Spades | Hearts | Clubs
type Card = Figure * Suit
type Rank =
| HighCard of Figure * Rank option
| Pair of Figure * Rank
| TwoPair of Figure Figure Rank
| ThreeOfKind of Figure
| Straight of Figure
| Flush of Rank
| FullHouse of Figure
| FourOfKind of Figure
| StraightFlush of Rank
type Player = {
Name: string;
Cards: Card list;
Hand: Rank;
}
let SortDesc sequence = sequence |> Seq.toList |> List.sortWith (fun x y -> compare y x)
let SortDescBy f sequence = sequence |> List.sortWith (fun x y -> compare (f y) (f x))
let rec BuildHighCard (figures : Figure list) =
match SortDesc figures with
| [h] -> HighCard(h, None)
| h :: t -> HighCard(h, Some(BuildHighCard(t)))
| _ -> failwith "empty"
let isFlush (cards : Card list) =
cards |> Seq.distinctBy snd |> Seq.length = 1
let isStraight (cards : Card list) =
let figs = cards |> List.map fst |> List.sort
let flag = ref true
for i in {1 .. List.length figs - 1} do
if (compare figs.[i-1] figs.[i]) <> -1 then flag.Value List.map fst))) else None
let (|IsFlush|_|) cards =
if isFlush cards then Some(Flush(BuildHighCard(cards |> List.map fst))) else None
let (|IsStraight|_|) cards =
if isStraight cards then Some(Straight(cards |> List.minBy fst |> fst)) else None
let (|IsGrouped|_|) (counts : int list) cards =
let groups = cards |> Seq.countBy fst |> Seq.toList |> SortDescBy snd
if groups |> List.map snd = counts then Some(groups |> List.map fst) else None
let DetermineRank (cards : Card list) =
match cards with
| IsStraightFlush straightFlus
Solution
I like this! Your use of discriminated unions and active patterns is quite elegant, I think. (Although exploiting that
As to your concerns:
-
Here is a manual way to do it functionally (replace the
For more on this style, see here. Alternatively, you could try to use combinators. If you're willing to compare only sequences of five cards, then maybe like this?
-
I think these are fine. I don't think you should try to factor out the redundant looking
-
Yes:
But maybe already
Minor suggestion: You could implement
In general, when you write
you can cut out the middle man and just do
compare on discriminated unions does what it does seems a bit of a hack–although I have a hard time justifying that sentiment.)As to your concerns:
-
Here is a manual way to do it functionally (replace the
let flag ... flag.Value with this):let rec check = function
| x :: y :: zs -> compare x y <> -1 && check (y :: zs)
| [x] -> true
| _ -> false
check figsFor more on this style, see here. Alternatively, you could try to use combinators. If you're willing to compare only sequences of five cards, then maybe like this?
List.toSeq cards |> Seq.distinct |> Seq.toList
|> function
| [x;_;_;_;y] -> compare x y = -4
| _ -> false-
I think these are fine. I don't think you should try to factor out the redundant looking
BuildHighCard(cards |> List.map fst). I think if you did, you'd clutter your program rather than make it more readable. As it is, it's perfectly clear what's going on, yes?-
Yes:
let winner, loser = Array.max players, Array.min playersBut maybe already
ParsePlayers return a tuple instead of an array?Minor suggestion: You could implement
SortDesc and SortDescBy as:let SortDescBy f = List.sortWith (fun x y -> compare (f y) (f x))In general, when you write
x |> f it means just f x. So when you definelet f x = x |> g ...you can cut out the middle man and just do
let f = g ...Code Snippets
let rec check = function
| x :: y :: zs -> compare x y <> -1 && check (y :: zs)
| [x] -> true
| _ -> false
check figsList.toSeq cards |> Seq.distinct |> Seq.toList
|> function
| [x;_;_;_;y] -> compare x y = -4
| _ -> falselet winner, loser = Array.max players, Array.min playerslet SortDescBy f = List.sortWith (fun x y -> compare (f y) (f x))let f x = x |> g ...Context
StackExchange Code Review Q#43831, answer score: 6
Revisions (0)
No revisions yet.