From 692e430bf98589936c10b8ad11a5163f9a914402 Mon Sep 17 00:00:00 2001 From: Your Name Date: Thu, 19 Feb 2026 10:24:44 -0800 Subject: [PATCH] Fix 0/1 Knapsack traceback crash and incorrect item selection The traceback phase of solveZeroOneKnapsackProblem() had two bugs: 1. Accessed knapsackMatrix[itemIndex - 2] without bounds checking, causing "Cannot read property of undefined" when itemIndex was 1. 2. Used incorrect logic that pushed prevItem instead of currentItem when an item was included, leading to wrong selected items. Replaced with the standard DP traceback: if dp[i][w] != dp[i-1][w], item i was included. Also added check for the first item (row 0). Added test case from issue #248 that previously crashed. Fixes #248 --- .../sets/knapsack-problem/Knapsack.js | 30 ++++++------------- .../__test__/Knapsack.test.js | 19 ++++++++++++ 2 files changed, 28 insertions(+), 21 deletions(-) diff --git a/src/algorithms/sets/knapsack-problem/Knapsack.js b/src/algorithms/sets/knapsack-problem/Knapsack.js index 5d1491110..dacaaeabb 100644 --- a/src/algorithms/sets/knapsack-problem/Knapsack.js +++ b/src/algorithms/sets/knapsack-problem/Knapsack.js @@ -122,32 +122,20 @@ export default class Knapsack { while (itemIndex > 0) { const currentItem = this.possibleItems[itemIndex]; - const prevItem = this.possibleItems[itemIndex - 1]; - - // Check if matrix value came from top (from previous item). - // In this case this would mean that we need to include previous item - // to the list of selected items. - if ( - knapsackMatrix[itemIndex][weightIndex] - && knapsackMatrix[itemIndex][weightIndex] === knapsackMatrix[itemIndex - 1][weightIndex] - ) { - // Check if there are several items with the same weight but with the different values. - // We need to add highest item in the matrix that is possible to get the highest value. - const prevSumValue = knapsackMatrix[itemIndex - 1][weightIndex]; - const prevPrevSumValue = knapsackMatrix[itemIndex - 2][weightIndex]; - if ( - !prevSumValue - || (prevSumValue && prevPrevSumValue !== prevSumValue) - ) { - this.selectedItems.push(prevItem); - } - } else if (knapsackMatrix[itemIndex - 1][weightIndex - currentItem.weight]) { - this.selectedItems.push(prevItem); + + // If the value differs from the row above, the current item was included. + if (knapsackMatrix[itemIndex][weightIndex] !== knapsackMatrix[itemIndex - 1][weightIndex]) { + this.selectedItems.push(currentItem); weightIndex -= currentItem.weight; } itemIndex -= 1; } + + // Check if the first item was also included. + if (knapsackMatrix[0][weightIndex] !== 0) { + this.selectedItems.push(this.possibleItems[0]); + } } // Solve unbounded knapsack problem. diff --git a/src/algorithms/sets/knapsack-problem/__test__/Knapsack.test.js b/src/algorithms/sets/knapsack-problem/__test__/Knapsack.test.js index d322445a7..8a5918d2c 100644 --- a/src/algorithms/sets/knapsack-problem/__test__/Knapsack.test.js +++ b/src/algorithms/sets/knapsack-problem/__test__/Knapsack.test.js @@ -44,6 +44,25 @@ describe('Knapsack', () => { expect(knapsack.selectedItems[1].toString()).toBe('v4 w3 x 1'); }); + it('should solve 0/1 knapsack problem with no single-weight-1 item (issue #248)', () => { + const possibleKnapsackItems = [ + new KnapsackItem({ value: 3, weight: 2 }), + new KnapsackItem({ value: 4, weight: 3 }), + new KnapsackItem({ value: 5, weight: 4 }), + new KnapsackItem({ value: 7, weight: 5 }), + ]; + + const maxKnapsackWeight = 7; + + const knapsack = new Knapsack(possibleKnapsackItems, maxKnapsackWeight); + + knapsack.solveZeroOneKnapsackProblem(); + + expect(knapsack.totalValue).toBe(10); + expect(knapsack.totalWeight).toBe(7); + expect(knapsack.selectedItems.length).toBe(2); + }); + it('should solve 0/1 knapsack problem with impossible items set', () => { const possibleKnapsackItems = [ new KnapsackItem({ value: 5, weight: 40 }),