Harris M Snyder
Harris M Snyder

Reputation: 115

Idiomatic Haskell way of termination recursion in this functoin

What is the more haskellish way to stop the recursion of this function? Currently I'm using a nested if/else, and returning an empty list if the next combination "overflows".

nextcomb [] []     = []
nextcomb lst maxes | length lst == length maxes = 
let lastel = last lst
in if    lastel < last maxes
   then (init lst) ++ [lastel+1]
   else let higherbit = (nextcomb (init lst) (init maxes)) 
        in if   higherbit == [] 
           then []
           else higherbit ++ [1]
nextcomb lst maxes | otherwise = []

To clarify, what it does is it takes a list of numbers like [1,1,1,1] and increments it like:

[1,1,1,1] -> [1,1,1,2]

...

[1,1,1,9] -> [1,1,2,1]

...

[1,1,9,9] -> [1,2,1,1]

etc.

BUT, the second argument is a list that indicates the maxmum value for each column. So if the maxes were [2,3], and the initial list was [1,1], then the progression woudld be:

[1,1] -> [1,2]

[1,2] -> [1,3]

[1,3] -> [2,1]

[2,1] -> [2,2]

[2,2] -> [2,3]

[2,3] -> []


Edit: "Little Endian" version as recommended by chepner

nextcomb' [] [] = []
nextcomb' lst maxes | length lst /= length maxes = []
nextcomb' lst maxes =
    let firstel = head lst
    in if    firstel < head maxes
       then (firstel+1) : (tail lst)
       else let higherbit = (nextcomb' (tail lst) (tail maxes))
            in if   higherbit == []
               then []
               else 1 : higherbit

Upvotes: 1

Views: 186

Answers (4)

Luis Casillas
Luis Casillas

Reputation: 30237

Let's draw a diagram! I'm going to make slightly different assumptions than the initial problem:

  1. A little-endian representation like chepner suggested;
  2. Instead of inclusive maximum values, I'm going to go with exclusive bases to make things more similar to addition-with-carry.
  3. I'm going to use digits in the range [0, base).

Here's the diagram:

 digits = [d0, d1, ..., dn]
 bases  = [b0, b1, ..., bn]
 --------------------------
 result = [r0, r1, ..., rn]

Now we can ask: for each digit ri of the result, what does its value depend on? Well, these things:

  1. The value of di
  2. The value of bi
  3. Whether the previous r resulted in a carry

So we can write this as a function:

import Control.Monad.State  -- gonna use this a bit later

type Base = Int
type Digit = Int
type Carry = Bool

-- | Increment a single digit, given all the contextual information.
singleDigit' :: Base -> Digit -> Carry -> (Digit, Carry)
singleDigit' base digit carry = (digit', carry')
    where sum = digit + if carry then 1 else 0
          digit' = if sum < base then sum else sum - base
          carry' = base <= sum

Note that I took care to make sure that the singleDigit' function's type ends with Carry -> (Digit, Carry). This is because that fits the state -> (result, state) pattern that is typical of the state monad:

-- | Wrap the `singleDigit'` function into the state monad.
singleDigit :: Base -> Digit -> State Carry Digit
singleDigit base digit = state (singleDigit' base digit)

Now with that we can write the following function:

increment :: [Base] -> [Digit] -> [Digit]
increment bases digits = evalState (sequence steps) True
    where steps :: [State Carry Digit]
          steps = zipWith singleDigit bases digits

What we're doing here is:

  1. Using zipWith to "sew" the bases and digits list together on corresponding elements. The elements of this list correspond to the individual steps of the computation.
  2. Using sequence :: [State Carry Digit] -> State Carry [Digit] to chain all the individual steps into one big step that passes the intermediate Carry state around.
  3. Feeding True as the initial Carry input to that big chain of steps (which causes the chain to increment).

Example invocation:

>>> take 20 (iterate (increment [3,4,5,10]) [0,0,0,0])
[[0,0,0,0],[1,0,0,0],[2,0,0,0]
,[0,1,0,0],[1,1,0,0],[2,1,0,0]
,[0,2,0,0],[1,2,0,0],[2,2,0,0]
,[0,3,0,0],[1,3,0,0],[2,3,0,0]
,[0,0,1,0],[1,0,1,0],[2,0,1,0]
,[0,1,1,0],[1,1,1,0],[2,1,1,0]
,[0,2,1,0],[1,2,1,0]
]

The lessons I'd stress:

  1. Break problems into smaller pieces. Don't try solve too much in one function! In this case, the trick was to split off the solution for single digits into its own function.
  2. It pays very much to think carefully about the data flow in a problem: what information is needed at each step of the problem? In this case, the diagram helped reason that out, which led to the singleDigit' function.
  3. One of the big ideas of functional programming is to separate the "shape" of the computation from its "content". In this case, the "content" is the singleDigit operation, and the "shape" of the computation—how to combine the individual steps into a big solution—is provided by the State monad and the sequence operation.
  4. I did not write a single recursive function; instead, I made ample use of library functions like zipWith, sequence, take and iterate. You ask for a more idiomatic Haskell solution, so well, here goes: complicated recursive functions definitions are not as idiomatic as using library functions that encapsulate common recursive patterns.
  5. This hopefully will encourage you to study monads some more. There are many, many problems out there where, if you express them in terms of one of the standard monads like State, you can reuse generic functions like sequence to solve them very easily. It's a tall learning curve but the results are worth it!

Upvotes: 1

Ivan David
Ivan David

Reputation: 348

Please check if next implementation is useful for you, because a more "haskellish" way (at least for me), is using built in recursive functions to achieve the same goal

nextcomb [] [] = []
nextcomb lst maxes
  | length lst /= length maxes = []
  | lst == maxes = []
  | otherwise = fst $ foldr f ([],True) $ zip lst maxes
  where
    f (l,m) (acc, mustGrow)
      | mustGrow && l < m  = (l + 1:acc, False)
      | mustGrow = (1:acc, True)
      | otherwise = (l:acc, False)

(Edited) If need catch errors then can try this:

nextcomb [] _ = Left "Initial is empty"
nextcomb _ [] = Left "Maximus size are empty"
nextcomb lst maxes
  | length lst /= length maxes = Left "List must be same length"
  | lst == maxes = Left "Initial already reach the limit given by Maximus"
  | otherwise = Right $ fst $ foldr f ([],True) $ zip lst maxes
  where
    f (l,m) (acc, mustGrow)
      | mustGrow && l < m  = (l + 1:acc, False)
      | mustGrow = (1:acc, True)
      | otherwise = (l:acc, False)

Upvotes: 2

Ingo
Ingo

Reputation: 36339

You should make illegal states unrepresentable

So, instead of using two lists, use a list of tuples. For example, the first value in each tuple could be the maximum, the second one the actual value.

This also simplifies the logic dramatically, as the errors "maxes too long" and "maxes too short" can not happen.

Upvotes: 3

chepner
chepner

Reputation: 531430

Your if expression is simply hiding the real base case, which is that if either argument is empty, return the empty list.

nextcomb [] [] = []
nextcomb lst maxes | length lst != length maxes = []
nextcomb lst maxes = let lastel = last lst
                     in if    lastel < last maxes
                        then (init lst) ++ [lastel+1]
                        else let higherbit = (nextcomb (init lst) (init maxes)) 
                             in if   higherbit == [] 
                                then []
                                else higherbit ++ [1]

I would probably rework the logic like this. (Note, I'm far from a Haskell expert and tend to answer these questions as an exercise for myself :)

-- Reversing the arguments and the ultimate return value
-- lets you work with the head of each list, rather than the last
-- element
nextcomb lst maxes = reverse $ nextcomb' (reverse lst) (reverse maxes)
-- The real work. The base case is two empty lists
nextcomb' [] [] = []
-- If one list runs out before the other, it's an error. I think
-- it's faster to check if one argument is empty when the other is not
-- than to check the length of each at each level of recursion.
nextcomb' [] _ = error "maxes too long"
nextcomb' _ [] = error "maxes too short"
-- Otherwise, it's just a matter of handling the least-significant
-- bit correctly. Either abort, increment, or reset and recurse
nextcomb' (x:xs)  (m:ms) | x > m = error "digit too large"
                         | x < m = (x+1):xs  -- just increment
                         | otherwise = 0:(nextcomb' xs ms) -- reset and recurse

(Actually, note that nextcomb' [] _ won't trigger if you don't recurse after the last digit. You could argue that a too-long maxes isn't a big deal. I'll leave this unfixed, as the next part handles it correctly.)

Alternately, you could verify the lengths match in the initial call; then you can assume that both will become empty at the same time.

nextcomb lst maxes | length lst == length maxes = reverse $ nextcomb' (reverse lst) (reverse maxes)
                   | otherwise = error "length mixmatch"

nextcomb' [] [] = []
nextcomb' (x:xs)  (m:ms) | x > m = error "digit too large"
                         | x < m = (x+1):xs
                         | otherwise = 0:(nextcomb' xs ms)

Here's an example using Either to report errors. I won't vouch for the design other than to say it does type check and run. It's not too different from the previous code; it just uses <$> to lift reverse and (0:) to work with arguments of type Either String [a] instead of arguments of type [a].

import Control.Applicative
nextcombE lst maxes = reverse <$> nextcombE' (reverse lst) (reverse maxes)

nextcombE' [] [] = Right []
nextcombE' [] _ = Left "maxes too long"
nextcombE' _ [] = Left "maxes too short"
nextcombE' (x:xs) (m:ms) | x > m = Left "digit too large"
                         | x < m = Right ((x+1):xs)
                         | otherwise = (0:) <$> (nextcombE' xs ms)

Upvotes: 2

Related Questions