I'm trying to solve a fractional knapsack problem with the input,
[("label 1", value, weight), ("label 2", value, weight), ...]
and the output,
[("label 1", value, solution_weight), ("label 2", value, solution_weight), ...]
but I keep getting the error in computeKnapsack
:
"Couldn't match expected type `(t1, t2, t0)' with actual type `[a0]`"
I can't understand what the problem might be. I'm trying to create a list of 3-entry tuples. How can I get it to stop expecting a single 3-entry tuple?
fst3 (a,b,c) = a
snd3 (a,b,c) = b
trd3 (a,b,c) = c
fst4 (a,b,c,d) = a
snd4 (a,b,c,d) = b
trd4 (a,b,c,d) = c
qud4 (a,b,c,d) = d
sortFrac (a1, b1, c1, d1) (a2, b2, c2, d2)
| d1 > d2 = GT
| d1 <= d2 = LT
fracKnap (x:xs) =
fractionalKnapsack (x:xs) []
fractionalKnapsack (x:xs) fracList =
if length (x:xs) <= 1
then computeKnapsack (sortBy sortFrac (((fst3 x),(snd3 x),(trd3 x),(snd3 x) / (trd3 x)):fracList)) 100
else fractionalKnapsack xs (((fst3 x),(snd3 x),(trd3 x),(snd3 x) / (trd3 x)):fracList)
computeKnapsack (x:xs) weightLeft =
if length (x:xs) <= 1
then (fst4 x, snd4 x, ((floor (weightLeft / (qud4 x)))*(qud4 x)))
else (fst4 x, snd4 x, ((floor (weightLeft / (qud4 x)))*(qud4 x))):(computeKnapsack xs (weightLeft-(floor (weightLeft / (qud4 x))*(qud4 x))))
In the then
branch of computeKnapsack
the result is a single 3-tuple, but in the else
branch it is a list of 3-tuples.
I'm going to rewrite computeKnapsack
for you. I'll start with your version with the error fixed:
computeKnapsack (x:xs) weightLeft =
if length (x:xs) <= 1
then [(fst4 x, snd4 x, ((floor (weightLeft / (qud4 x)))*(qud4 x)))]
else (fst4 x, snd4 x, ((floor (weightLeft / (qud4 x)))*(qud4 x))) : (computeKnapsack xs (weightLeft-(floor (weightLeft / (qud4 x))*(qud4 x))))
First, I'm going to say what happens if the first argument to computeKnapsack
is the empty list:
computeKnapsack [] _ = []
computeKnapsack (x:xs) weightLeft =
(fst4 x, snd4 x, ((floor (weightLeft / (qud4 x)))*(qud4 x))) : (computeKnapsack xs (weightLeft-(floor (weightLeft / (qud4 x))*(qud4 x))))
It enables us to get rid of the if
-test, making the code shorter overall and easier to understand.
Next, I'll deconstruct x
:
computeKnapsack [] _ = []
computeKnapsack ((a,b,_,d):xs) weightLeft =
(a, b, ((floor (weightLeft / d))*d)) : (computeKnapsack xs (weightLeft-(floor (weightLeft / d)*d)))
You might prefer to follow leftaroundabout's suggestion to create a record type with meaningful names instead. But if you do continue to use a tuple, deconstructing it by pattern matching is much clearer than using your fst4
, snd4
etc functions.
Again, the code is now shorter and easier to understand, though it might help if we used more meaningful names than a
, b
, and d
.
We can continue in this vein:
computeKnapsack [] _ = []
computeKnapsack ((a,b,_,d):xs) weightLeft = (a, b, weight) : (computeKnapsack xs (weightLeft - weight))
where weight = floor (weightLeft / d) * d
Here I've spotted that the same value was being calculated in two different places, and extracted that value into its own named variable. weight
only needs to be calculated once instead of twice, so computeKnapsack
is now marginally more efficient. More importantly, I now understand what computeKnapsack
is doing.
I understand that you're new to Haskell. Please take this as constructive suggestions on how you can write clearer Haskell code.