Bear in mind that I know nothing about the domain nor the existing code in ex-cdm-swaps, so this may be completely off-base and should be taken with a boatload of salt.
It looks to me like, going back to your original block of code:
cps <-
case (rds, ods) of
(Some r, None) -> generateResetPeriods cpds rds
(None, Some o) -> generateOtherPeriods cpds ods
(Some r, Some o) -> do
let r = generateResetPeriods cpds rds
let o = generateOtherPeriods cpds ods
-- sequence expression to return both r and o
return ...
(None, None) -> error "expecting calculation period"
both generateResetPeriods
and generateOtherPeriods
are going to walk down the list of values in cpds
and, for each element in there (possibly based on the values in rds
or ods
), generate a new element that has some properties added.
Simplifying a bit, let’s assume we have the following definitions:
data CPD = CPD { reset: Optional Int, other: Optional Int }
generateResetPeriods: [CPD] -> Optional Int -> [CPD]
generateResetPeriods cpds None = cpds
generateResetPeriods cpds (Some x) =
map (\cpd -> cpd with reset = Some x) cpds
generateOtherPeriods: [CPD] -> Optional Int -> [CPD]
generateOtherPeriods cpds None = cpds
generateOtherPeriods cpds (Some x) =
map (\cpd -> cpd with other = Some x) cpds
Obviously what the code is doing here is much more complicated and involves fetches, but if the issue is what I think it is, this should be enough to illustrate it.
This gives us the following situation:
cps <-
case (rds, ods) of
-- works as expected: all elements of cpds end up with
-- the reset field set to the given value
(Some _, None) -> generateResetPeriods cpds rds
-- works as expected: all elements of cpds end up with
-- the other field set to the given value
(None, Some _) -> generateOtherPeriods cpds ods
-- this doesn't work: applying both functions seems
-- to work well enough at first, but then, what can we
-- do with the results?
(Some _, Some _) -> do
let r = generateResetPeriods cpds rds
let o = generateOtherPeriods cpds ods
-- r and o cannot easily be combined: we have two
-- sets of results where we want one answer
return ...
(None, None) -> error "blow up"
So what to do? In this very simplified case, the input and output types are the same, so we could easily resolve the issue by changing the (Some _, Some _)
case to:
let intermediateResult = generateResetPeriods cpds rds
let finalResult = generateOtherPeriods intermediateResult ods
return finalResult
But that doesn’t work in your case because the input and output types don’t match. Also, you have a Fetch
instance there. First, we could get rid of the Fetch
instance by turning the let
s into <-
. Second, we need a way to combine the results. Let’s update our example to match your case a bit better:
data Input = Input {}
data Output = Output { reset: Optional Int, other: Optional Int }
class (Action f) => Fetch f
generateResetPeriods: (Fetch f) => [Input] -> Optional Int -> f [Output]
generateResetPeriods cpds None =
return $ map (const $ Output None None) cpds
generateResetPeriods cpds (Some x) =
return $ map (const $ Output with other = None, reset = Some x) cpds
generateOtherPeriods: (Fetch f) => [Input] -> Optional Int -> f [Output]
generateOtherPeriods cpds None =
return $ map (const $ Output None None) cpds
generateOtherPeriods cpds (Some x) =
return $ map (const $ Output with other = Some x, reset = None) cpds
The (Some _, Some _)
case would become:
(Some _, Some _) -> do
let r1 = generateResetPeriods cpds rds
let r2 = generateOtherPeriods cpds ods
-- r1 and r2 have the same type here: f [Output]
-- what to do about that `f`?
First, let’s take care of the Fetch
issue: we would much rather deal with pure lists if possible. In this example, [Output]
, in your case, [CalculationPeriod]
. As mentioned above, we can do that by using <-
notation:
(Some _, Some _) -> do
r1 <- generateResetPeriods cpds rds
r2 <- generateOtherPeriods cpds ods
-- r1 and r2 have the same type here: [Output]
-- we still have two lists and we want only one
I can see two options here. The first one is what @cocreature was suggesting: traverse the original list only once. In the case of my simple example here, this would amount to rewriting the two generate
functions to something like:
import DA.Action((>=>))
data Input = Input {}
data Output = Output { reset: Optional Int, other: Optional Int }
class (Action f) => Fetch f
commonStep: (Fetch f) => Input -> f Output
commonStep i = return $ Output with other = None, reset = None
-- note: swapped arguments for notational convenience later on
resetStep: (Fetch f) => Optional Int -> Output -> f Output
resetStep None o = return o
resetStep (Some x) o = return $ o with reset = Some x
otherStep : (Fetch f) => Optional Int -> Output -> f Output
otherStep None o = return o
otherStep (Some x) o = return $ o with other = Some x
-- By having decomposed things in this way, we can write:
f: (Fetch f) => [Input] -> Optional Int -> Optional Int -> f [Output]
f cpds rds ods = do
case (rds, ods) of
(Some _, None) -> mapA (commonStep >=> resetStep rds) cpds
(None, Some _) -> mapA (commonStep >=> otherStep ods) cpds
(Some _, Some _) ->
mapA (commonStep >=> resetStep rds >=> otherStep ods) cpds
(None, None) -> error "blow up"
This, however, means a lot of changes to the code as you need to extract a lot of logic from the guts of generateResetPeriods
and generateOtherPeriods
. Another option (which may or may not be applicable depending on some details of what these functions do that I don’t fully understand) is to combine the results of those functions after the fact, rather than combine their application. This can only be done if both functions do, as I believe, return lists of essentially the same things in the same order, just with some of them decorated with additional data.
In that case, the simplified example code becomes:
data Input = Input {}
data Output = Output { reset: Optional Int, other: Optional Int }
class (Action f) => Fetch f
generateResetPeriods: (Fetch f) => [Input] -> Optional Int -> f [Output]
generateResetPeriods cpds None =
return $ map (const $ Output None None) cpds
generateResetPeriods cpds (Some x) =
return $ map (const $ Output with other = None, reset = Some x) cpds
generateOtherPeriods: (Fetch f) => [Input] -> Optional Int -> f [Output]
generateOtherPeriods cpds None =
return $ map (const $ Output None None) cpds
generateOtherPeriods cpds (Some x) =
return $ map (const $ Output with other = Some x, reset = None) cpds
combine: Output -> Output -> Output
combine o1 o2 = Output with reset = o1.reset, other = o2.reset
f: (Fetch f) => [Input] -> Optional Int -> Optional Int -> f [Output]
f cpds rds ods = do
case (rds, ods) of
-- no change
(Some _, None) -> generateResetPeriods cpds rds
-- no change
(None, Some _) -> generateOtherPeriods cpds ods
(Some _, Some _) -> do
r1 <- generateResetPeriods cpds rds
r2 <- generateOtherPeriods cpds ods
-- the <- got rid of the fetches so we have two lists of results,
-- we just need to combine them by walking through both lists
-- at the same time
return $ zipWith combine r1 r2
(None, None) -> error "blow up"
Hope this helps.