Re: [Haskell-beginners] Re: Converting an imperative program to haskell

29 Apr 2010

      Am Donnerstag 29 April 2010 23:49:16 schrieb Hein Hundal:
...
--- On Thu, 4/29/10, Maciej Piechotka  wrote:
...
Hein Hundal wrote:
...
I figured I should try a larger program
in Haskell, so I am
...
converting one of my Mathematica programs, a simulator
for the
...
card game Dominion, over to Haskell.  Most of
that is going well
...
except for one patch of imperative code.  My
Haskell version of
...
this code is ugly.  I was hoping someone could
recommend a better
...
way to do it.  I will paste a simplified version
of the code
...
below.  If necessary, I can provide all the other
code used for
1. Use strong typing. Or any typing
I simplified the code for the post.  In the real version, I use strong
typing.  The Card type is enumerated.  I have been using [Card] instead
of calling it a Deck.  I could change that.
Whether you use [Card],

type Deck = [Card]

or

newtype Deck = D [Card]

is not important. Using enumerations for suite and value instead of Ints 
is.
...
...
2. User guards instead of nested if's:
In the "procloop" function, I have if's nested quite deeply.
Nested ifs (beyond, say, three levels) are a pain to decipher. Using guards 
(and maybe 'case's) will probably be a great improvement.
...
I could easily use guards.  I will try that.
...
3. Don't use too much variables. 6-8 is probably the
maximum you should
deal with (human short-term memory holds about 5-10 points
of entry.
Split functions into smaller functions (even in where).
I do have to get the information into the functions, so the only way I
can avoid having lots of variables is by introducing new structures.
I can do that.
As long as they're meaningful. Just smashing a couple of values together 
into a structure to reduce the variable count isn't good.
...
...
4. Discard result you don't need:
isDone  (LSt gs strat iA iTh i aA iB iC bDone) =
bDone
isDone  (LSt _ _ _ _ _ _ _ _ _ bDone) = bDone
Yes, that's much better.
Perhaps use named field syntax,

data LoopState
    = LSt
    { gameState :: GameState
    , strat :: Strategy
    , ...
    , isDone :: Bool
    }

Then in procloop, where you just update one or two fields,

procloop lst@(LSt gs' strat iA iTh i aA iB iC bDone)
    | iA<=0 || i>=20 || actcd gs <=0 || iCd == -1
        = lst{ idx = idx lst + 1, isDone = True }
    | iCd == 1
        = lst{ idx = idx lst + 1, iThFld = iThFld lst + 1, isDone = False }
    | iThFld lst > 0
        = lst{ ... }
    | otherwise
       = lst{ ... }
      where
        iCd = stratchoose ...
        -- other let-bindings, only those needed will be evaluated
...
proc gs' strat = let
      iA     = 1
      iTh    = 0
      i      = 0
      aA     = replicate 20 0
      iB     = 1
      iC     = 0
      bDone  = False
      gs     = apps ("<<<"++show(gs')++">>>") gs'
      lst    = LSt gs strat iA iTh i aA iB iC bDone
      lst2   = until isDone procloop lst
      isDone  (LSt _  _ _ _ _ _ _  _ bDone) = bDone
      output  (LSt gs _ _ _ _ aA iB iC _) = (iB, iC, appd aA gs)
  in output lst2
...
5. Don't reuse the same name in the same scope (like iA as
1 and iA as parameter to isDone).
I did hesitate to use the same parameter name twice.
...
6. While Haskell have long tradition of having short
namesit is not
always good (see 3). Use them only if you are sure it is
clear what they
mean:
In the original version, I had longer variable names where they seemed
necessary.
The main sources of ugliness are the long lists of variables.  Every
time I call doAct or construct a LoopState variable, I am repeating all
those variables.  I will try changing the type of doAct to
doAct :: LoopState -> LoopState
That looks promising.
...
Cheers,
Hein

Re: [Haskell-beginners] Re: Converting an imperative program to haskell

Daniel Fischer