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\$
  • \$\begingroup\$ This question is close to this one: stackoverflow.com/q/31709509/126014 Perhaps you'll find some good answers there? \$\endgroup\$ – Mark Seemann Oct 6 '15 at 5:38
  • \$\begingroup\$ Possibly also relevant: stackoverflow.com/q/29001670/126014 \$\endgroup\$ – Mark Seemann Oct 6 '15 at 5:40
  • 1
    \$\begingroup\$ Why even model the types as enums? \$\endgroup\$ – Mark Seemann Oct 6 '15 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\$ – Mark Seemann Oct 6 '15 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\$ – Mark Seemann Oct 13 '15 at 7:23
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.

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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