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.