From 9903478d3ea5daec9510b7cf57a45b1ba8189331 Mon Sep 17 00:00:00 2001 From: AJ Isaacs Date: Sat, 21 Mar 2026 17:07:43 -0400 Subject: [PATCH] refactor: simplify BestCombination.FindFrom2 and add tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove redundant early-return branches and unify loop body — Floor(remaining/length2) already returns 0 when remaining < length2, so both branches collapse into one. 14 tests cover all edge cases. Co-Authored-By: Claude Opus 4.6 (1M context) --- OpenNest.Engine/Fill/BestCombination.cs | 74 +++--------- OpenNest.Tests/BestCombinationTests.cs | 150 ++++++++++++++++++++++++ 2 files changed, 165 insertions(+), 59 deletions(-) create mode 100644 OpenNest.Tests/BestCombinationTests.cs diff --git a/OpenNest.Engine/Fill/BestCombination.cs b/OpenNest.Engine/Fill/BestCombination.cs index 69bb186..7c9a193 100644 --- a/OpenNest.Engine/Fill/BestCombination.cs +++ b/OpenNest.Engine/Fill/BestCombination.cs @@ -7,74 +7,30 @@ namespace OpenNest public static bool FindFrom2(double length1, double length2, double overallLength, out int count1, out int count2) { overallLength += Tolerance.Epsilon; - - if (length1 > overallLength) - { - if (length2 > overallLength) - { - count1 = 0; - count2 = 0; - return false; - } - - count1 = 0; - count2 = (int)System.Math.Floor(overallLength / length2); - return true; - } - - if (length2 > overallLength) - { - count1 = (int)System.Math.Floor(overallLength / length1); - count2 = 0; - return true; - } - - var maxCountLength1 = (int)System.Math.Floor(overallLength / length1); - - count1 = maxCountLength1; + count1 = 0; count2 = 0; - var remnant = overallLength - maxCountLength1 * length1; + var maxCount1 = (int)System.Math.Floor(overallLength / length1); + var bestRemnant = overallLength + 1; - if (remnant.IsEqualTo(0)) - return true; - - for (int countLength1 = 0; countLength1 <= maxCountLength1; ++countLength1) + for (var c1 = 0; c1 <= maxCount1; c1++) { - var remnant1 = overallLength - countLength1 * length1; + var remaining = overallLength - c1 * length1; + var c2 = (int)System.Math.Floor(remaining / length2); + var remnant = remaining - c2 * length2; - if (remnant1 >= length2) - { - var countLength2 = (int)System.Math.Floor(remnant1 / length2); - var remnant2 = remnant1 - length2 * countLength2; + if (!(remnant < bestRemnant)) + continue; - if (!(remnant2 < remnant)) - continue; + count1 = c1; + count2 = c2; + bestRemnant = remnant; - count1 = countLength1; - count2 = countLength2; - - if (remnant2.IsEqualTo(0)) - break; - - remnant = remnant2; - } - else - { - if (!(remnant1 < remnant)) - continue; - - count1 = countLength1; - count2 = 0; - - if (remnant1.IsEqualTo(0)) - break; - - remnant = remnant1; - } + if (remnant.IsEqualTo(0)) + break; } - return true; + return count1 > 0 || count2 > 0; } } } diff --git a/OpenNest.Tests/BestCombinationTests.cs b/OpenNest.Tests/BestCombinationTests.cs new file mode 100644 index 0000000..db0bd3b --- /dev/null +++ b/OpenNest.Tests/BestCombinationTests.cs @@ -0,0 +1,150 @@ +namespace OpenNest.Tests; + +public class BestCombinationTests +{ + [Fact] + public void BothFit_FindsZeroRemnant() + { + // 100 = 0*30 + 5*20 (algorithm iterates from countLength1=0, finds zero remnant first) + var result = BestCombination.FindFrom2(30, 20, 100, out var c1, out var c2); + + Assert.True(result); + Assert.Equal(0.0, 100.0 - (c1 * 30.0 + c2 * 20.0), 5); + } + + [Fact] + public void OnlyLength1Fits_ReturnsMaxCount1() + { + var result = BestCombination.FindFrom2(10, 200, 50, out var c1, out var c2); + + Assert.True(result); + Assert.Equal(5, c1); + Assert.Equal(0, c2); + } + + [Fact] + public void OnlyLength2Fits_ReturnsMaxCount2() + { + var result = BestCombination.FindFrom2(200, 10, 50, out var c1, out var c2); + + Assert.True(result); + Assert.Equal(0, c1); + Assert.Equal(5, c2); + } + + [Fact] + public void NeitherFits_ReturnsFalse() + { + var result = BestCombination.FindFrom2(100, 200, 50, out var c1, out var c2); + + Assert.False(result); + Assert.Equal(0, c1); + Assert.Equal(0, c2); + } + + [Fact] + public void Length1FillsExactly_ZeroRemnant() + { + var result = BestCombination.FindFrom2(25, 10, 100, out var c1, out var c2); + + Assert.True(result); + Assert.Equal(0.0, 100.0 - (c1 * 25.0 + c2 * 10.0), 5); + } + + [Fact] + public void MixMinimizesRemnant() + { + // 7 and 3 into 20: best is 2*7 + 2*3 = 20 (zero remnant) + var result = BestCombination.FindFrom2(7, 3, 20, out var c1, out var c2); + + Assert.True(result); + Assert.Equal(2, c1); + Assert.Equal(2, c2); + Assert.True(c1 * 7 + c2 * 3 <= 20); + } + + [Fact] + public void PrefersLessRemnant_OverMoreOfLength1() + { + // 6 and 5 into 17: + // all length1: 2*6=12, remnant=5 -> actually 2*6+1*5=17 perfect + var result = BestCombination.FindFrom2(6, 5, 17, out var c1, out var c2); + + Assert.True(result); + Assert.Equal(0.0, 17.0 - (c1 * 6.0 + c2 * 5.0), 5); + } + + [Fact] + public void EqualLengths_FillsWithLength1() + { + var result = BestCombination.FindFrom2(10, 10, 50, out var c1, out var c2); + + Assert.True(result); + Assert.Equal(5, c1 + c2); + } + + [Fact] + public void SmallLengths_LargeOverall() + { + var result = BestCombination.FindFrom2(3, 7, 100, out var c1, out var c2); + + Assert.True(result); + var used = c1 * 3.0 + c2 * 7.0; + Assert.True(used <= 100); + Assert.True(100 - used < 3); // remnant less than smallest piece + } + + [Fact] + public void Length2IsBetter_SoleCandidate() + { + // length1=9, length2=5, overall=10: + // length1 alone: 1*9=9 remnant=1 + // length2 alone: 2*5=10 remnant=0 + var result = BestCombination.FindFrom2(9, 5, 10, out var c1, out var c2); + + Assert.True(result); + Assert.Equal(0, c1); + Assert.Equal(2, c2); + } + + [Fact] + public void FractionalLengths_WorkCorrectly() + { + var result = BestCombination.FindFrom2(2.5, 3.5, 12, out var c1, out var c2); + + Assert.True(result); + var used = c1 * 2.5 + c2 * 3.5; + Assert.True(used <= 12.0 + 0.001); + } + + [Fact] + public void OverallExactlyOneOfEach() + { + var result = BestCombination.FindFrom2(40, 60, 100, out var c1, out var c2); + + Assert.True(result); + Assert.Equal(1, c1); + Assert.Equal(1, c2); + } + + [Fact] + public void OverallSmallerThanEither_ReturnsFalse() + { + var result = BestCombination.FindFrom2(10, 20, 5, out var c1, out var c2); + + Assert.False(result); + Assert.Equal(0, c1); + Assert.Equal(0, c2); + } + + [Fact] + public void ZeroRemnant_StopsEarly() + { + // 4 and 6 into 24: 0*4+4*6=24 or 3*4+2*6=24 or 6*4+0*6=24 + // Algorithm iterates from 0 length1 upward, finds zero remnant and breaks + var result = BestCombination.FindFrom2(4, 6, 24, out var c1, out var c2); + + Assert.True(result); + Assert.Equal(0.0, 24.0 - (c1 * 4.0 + c2 * 6.0), 5); + } +}