Reputation: 115
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
Reputation: 30237
Let's draw a diagram! I'm going to make slightly different assumptions than the initial problem:
[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:
di
bi
r
resulted in a carrySo 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:
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.sequence :: [State Carry Digit] -> State Carry [Digit]
to chain all the individual steps into one big step that passes the intermediate Carry
state around.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:
singleDigit'
function.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.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.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
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
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
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