1
\$\begingroup\$
type Suit = Club = 1 | Diamond = 2 | Spade = 3 | Heart = 4
type Rank = Two = 2 | Three = 3 | Four = 4 | Five = 5 | Six = 6 | Seven = 7 | Eight = 8 | Nine = 9 | Ten = 10 | Jack = 11 | Queen = 12 | King = 13 | Ace = 14
type Card = { Suit:Suit; Rank:Rank }

let RandomHand (size:int) =            
    let rndSuitIndex (rng:System.Random) = rng.Next(1, System.Enum.GetValues(typeof<Suit>).Length + 1)
    let rndRankIndex (rng:System.Random) = rng.Next(1, System.Enum.GetValues(typeof<Rank>).Length + 2)            
    let randomCard (rng:System.Random) = {Suit=enum<Suit>((rndSuitIndex rng));Rank=enum<Rank>((rndRankIndex rng))}
    let cards (rng:System.Random) = List.init<Card> size (fun index -> randomCard rng)
    cards (System.Random())

As a C# programmer I'm trying to get into F#. I'm trying to create a function that returns a random list of cards. This currently works, but I'm sure this can be written MUCH shorter and cleaner.

Especially using the (rng:System.Random) 4 times feels like a stupid thing to do. Any tips?

\$\endgroup\$
9
  • \$\begingroup\$ This question is close to this one: stackoverflow.com/q/31709509/126014 Perhaps you'll find some good answers there? \$\endgroup\$ Commented Oct 6, 2015 at 5:38
  • \$\begingroup\$ Possibly also relevant: stackoverflow.com/q/29001670/126014 \$\endgroup\$ Commented Oct 6, 2015 at 5:40
  • 1
    \$\begingroup\$ Why even model the types as enums? \$\endgroup\$ Commented Oct 6, 2015 at 6:40
  • 1
    \$\begingroup\$ Regarding the randomness: why not pick a random number between 1 and 52, and map that to a card? \$\endgroup\$ Commented Oct 6, 2015 at 8:46
  • 1
    \$\begingroup\$ Why not create the entire deck? It's only 52 immutable values. Given a full deck, you can shuffle it as described here and pull the head off the list. \$\endgroup\$ Commented Oct 13, 2015 at 7:23

1 Answer 1

1
\$\begingroup\$

I second Mark Seemann's remark regarding enums. It makes much more sense to use discriminated unions to describe the entities in your domain.

A randomizing function will never look completely nice and functional, as it uses a side-effect by definition. Nevertheless, I think there isn't much point in passing around the random seed. It can be initialized once in the beginning of your function.

I think things get much clearer when the function randomHand has the call to List.init as its last line, showing that it creates and returns a list. The card generating function could in principle be made into a lambda, but I couldn't squeeze it into less than 7 lines and fslint doesn't like it...

Here's my suggestion:

type Suit = Club | Diamond | Spade | Heart
type Rank = Two | Three | Four | Five | Six | Seven | Eight | Nine | Ten | Jack | Queen | King | Ace
type Card = 
  { Suit: Suit 
    Rank: Rank }

let randomHand size =
  let randomSeed = System.Random ()
  let randomCard _ =
    { Suit = match randomSeed.Next(1, 5) with
             | 1 -> Club | 2 -> Diamond | 3 -> Spade | 4 -> Heart
             | _ -> failwith "random value out of bounds"
      Rank = match randomSeed.Next(2, 15) with
             | 2  -> Two   | 3  -> Three | 4  -> Four | 5  -> Five | 6  -> Six
             | 7  -> Seven | 8  -> Eight | 9  -> Nine | 10 -> Ten  | 11 -> Jack
             | 12 -> Queen | 13 -> King  | 14 -> Ace
             | _ -> failwith "random value out of bounds" }
  List.init size randomCard

Since System.Random.Next() is not bound to a range by its type (int), I had to handle the potential case that it might return a value out of bounds.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.