Yacht: Dig Deeper Solution Fails "Four of a Kind" Test

The solutions in the “Dig deeper” section of the “Yacht” exercise fails when the same number is at indices 0, 2, 3, and 4, with any other number at index 1.

Test file:

# These tests are auto-generated with test data from:
# https://github.com/exercism/problem-specifications/tree/main/exercises/yacht/canonical-data.json
# File last updated on 2023-07-19

import unittest
import yacht


class YachtTest(unittest.TestCase):

    // snip

    def test_four_of_a_kind_one(self):
        self.assertEqual(yacht.score([4, 6, 6, 6, 6], yacht.FOUR_OF_A_KIND), 24)

    def test_four_of_a_kind_two(self):    // FAILS - returns 16
        self.assertEqual(yacht.score([6, 4, 6, 6, 6], yacht.FOUR_OF_A_KIND), 24)

    def test_four_of_a_kind_three(self):
        self.assertEqual(yacht.score([6, 6, 4, 6, 6], yacht.FOUR_OF_A_KIND), 24)

    def test_four_of_a_kind_four(self):
        self.assertEqual(yacht.score([6, 6, 6, 4, 6], yacht.FOUR_OF_A_KIND), 24)

    def test_four_of_a_kind_five(self):
        self.assertEqual(yacht.score([6, 6, 6, 6, 4], yacht.FOUR_OF_A_KIND), 24)

    // snip

1 Like

I assume you’re referring to this line?

FOUR_OF_A_KIND = lambda dice: 4 * dice[1] if dice[0] == dice[3] or dice[1] == dice[4] else 0

I was initially referring to the structural pattern matching approach, which has

case 'FOUR_OF_A_KIND' if dice[0] == dice[3] or dice[1] == dice[4]:
            return dice[1] * 4

but looks like other approaches - lambda functions and if structure also have similar code.

@saurabh-mish – What version of the tests do you have? I just tried all of those solutions in the online editor, and all tests passed for me. But we may have different versions of the exercise.

Could you paste in the test case that is failing, and also the version of Python that you’re using? Many thanks!.

edited to add: Apologies! I see you’ve pasted in some of the tests. Let me investigate. :slightly_smiling_face:

At the moment, there’s only one “regular” test case for FOUR_OF_A_KIND.

I wanted to dig deeper into the exercise and noticed that the current explanations for this particular case don’t look good with any approach.

So I added all possible “regular” test cases for this condition locally and saw the error.

So the code passes the test cases of record, but not the test case you added?

Yes, the test cases I added locally (pasted above) don’t pass. All my tests are based on the description.

So what is your proposal here? The approaches as written do pass all tests as published, and are not incorrect. They do fail in the case where the dice need to be sorted (arguably to account for all possible cases of 4_of_a_kind).

Are you proposing a change to the approaches code, or to the explanation?

So what is your proposal here?

My proposal is to change

case 'FOUR_OF_A_KIND' if dice[0] == dice[3] or dice[1] == dice[4]:
    return dice[1] * 4

to

case 'FOUR_OF_A_KIND': return sum([x for x in dice if dice.count(x) >= 4][:4])

in all dig deeper sections, because it passes all existing tests and the ones that I’ve pasted.

The approaches as written do pass all tests as published, and are not incorrect.

They’re not incorrect, but they’re insufficient.

Just for reference, here are the problematic lines from the two solutions from the “Dig Deeper” tab:

  1.  case 'FOUR_OF_A_KIND' if dice[0] == dice[3] or dice[1] == dice[4]:
         return dice[1] * 4
    
    source
  2.  elif category == 'FOUR_OF_A_KIND':
         if dice[0] == dice[3] or dice[1] == dice[4]:
             return dice[1] * 4 or 0
    
    source

They do indeed not check if four of the dice are equal, they just happen to pass the tests.

I think the lines all fail to sort the dice, so the checks as written won’t hold for all orderings. They will, if the dice are sorted.
But the solution can also be changed to check if 4 are equal. I don’t think either variant is slower than the other (doing a sort and checking, or doing an equality count.), but if someone is motivated, they can check that.

I think the lines all fail to sort the dice, so the checks as written won’t hold for all orderings. They will, if the dice are sorted.

I dont think this assumption can be made in any approach in the dig deeper section.

case 'FOUR_OF_A_KIND': return sum([x for x in dice if dice.count(x) >= 4][:4])

is better than

case 'FOUR_OF_A_KIND' if dice[0] == dice[3] or dice[1] == dice[4]:
    return dice[1] * 4

because it passes all possible tests for this scenario.

Actually this doesn’t pass all possible tests (pasted above) for “FOUR_OF_A_KIND”.

I agree with you. I just pasted the problematic lines and their source for easier reference.

1 Like

Here’s the complete test file I’m using locally:

# These tests are auto-generated with test data from:
# https://github.com/exercism/problem-specifications/tree/main/exercises/yacht/canonical-data.json
# File last updated on 2023-07-19

import unittest
import yacht


class YachtTest(unittest.TestCase):
    def test_yacht(self):
        self.assertEqual(yacht.score([5, 5, 5, 5, 5], yacht.YACHT), 50)

    def test_not_yacht(self):
        self.assertEqual(yacht.score([1, 3, 3, 2, 5], yacht.YACHT), 0)

    def test_ones(self):
        self.assertEqual(yacht.score([1, 1, 1, 3, 5], yacht.ONES), 3)

    def test_ones_out_of_order(self):
        self.assertEqual(yacht.score([3, 1, 1, 5, 1], yacht.ONES), 3)

    def test_no_ones(self):
        self.assertEqual(yacht.score([4, 3, 6, 5, 5], yacht.ONES), 0)

    def test_twos(self):
        self.assertEqual(yacht.score([2, 3, 4, 5, 6], yacht.TWOS), 2)

    def test_fours(self):
        self.assertEqual(yacht.score([1, 4, 1, 4, 1], yacht.FOURS), 8)

    def test_yacht_counted_as_threes(self):
        self.assertEqual(yacht.score([3, 3, 3, 3, 3], yacht.THREES), 15)

    def test_yacht_of_3s_counted_as_fives(self):
        self.assertEqual(yacht.score([3, 3, 3, 3, 3], yacht.FIVES), 0)

    def test_fives(self):
        self.assertEqual(yacht.score([1, 5, 3, 5, 3], yacht.FIVES), 10)

    def test_fives(self):
        self.assertEqual(yacht.score([5, 1, 5, 2, 6], yacht.FIVES), 10)

    def test_sixes(self):
        self.assertEqual(yacht.score([2, 3, 4, 5, 6], yacht.SIXES), 6)

    def test_full_house_two_small_three_big(self):
        self.assertEqual(yacht.score([2, 2, 4, 4, 4], yacht.FULL_HOUSE), 16)

    def test_full_house_three_small_two_big(self):
        self.assertEqual(yacht.score([5, 3, 3, 5, 3], yacht.FULL_HOUSE), 19)

    def test_two_pair_is_not_a_full_house(self):
        self.assertEqual(yacht.score([2, 2, 4, 4, 5], yacht.FULL_HOUSE), 0)

    def test_four_of_a_kind_is_not_a_full_house(self):
        self.assertEqual(yacht.score([1, 4, 4, 4, 4], yacht.FULL_HOUSE), 0)

    def test_yacht_is_not_a_full_house(self):
        self.assertEqual(yacht.score([2, 2, 2, 2, 2], yacht.FULL_HOUSE), 0)

    def test_four_of_a_kind_one(self):
        self.assertEqual(yacht.score([4, 6, 6, 6, 6], yacht.FOUR_OF_A_KIND), 24)

    def test_four_of_a_kind_two(self):
        self.assertEqual(yacht.score([6, 4, 6, 6, 6], yacht.FOUR_OF_A_KIND), 24)

    def test_four_of_a_kind_three(self):
        self.assertEqual(yacht.score([6, 6, 4, 6, 6], yacht.FOUR_OF_A_KIND), 24)

    def test_four_of_a_kind_four(self):
        self.assertEqual(yacht.score([6, 6, 6, 4, 6], yacht.FOUR_OF_A_KIND), 24)

    def test_four_of_a_kind_five(self):
        self.assertEqual(yacht.score([6, 6, 6, 6, 4], yacht.FOUR_OF_A_KIND), 24)

    def test_yacht_can_be_scored_as_four_of_a_kind(self):
        self.assertEqual(yacht.score([3, 3, 3, 3, 3], yacht.FOUR_OF_A_KIND), 12)

    def test_full_house_is_not_four_of_a_kind(self):
        self.assertEqual(yacht.score([3, 3, 3, 5, 5], yacht.FOUR_OF_A_KIND), 0)

    def test_little_straight(self):
        self.assertEqual(yacht.score([3, 5, 4, 1, 2], yacht.LITTLE_STRAIGHT), 30)

    def test_little_straight_as_big_straight(self):
        self.assertEqual(yacht.score([1, 2, 3, 4, 5], yacht.BIG_STRAIGHT), 0)

    def test_four_in_order_but_not_a_little_straight(self):
        self.assertEqual(yacht.score([1, 1, 2, 3, 4], yacht.LITTLE_STRAIGHT), 0)

    def test_no_pairs_but_not_a_little_straight(self):
        self.assertEqual(yacht.score([1, 2, 3, 4, 6], yacht.LITTLE_STRAIGHT), 0)

    def test_minimum_is_1_maximum_is_5_but_not_a_little_straight(self):
        self.assertEqual(yacht.score([1, 1, 3, 4, 5], yacht.LITTLE_STRAIGHT), 0)

    def test_big_straight(self):
        self.assertEqual(yacht.score([4, 6, 2, 5, 3], yacht.BIG_STRAIGHT), 30)

    def test_big_straight_as_little_straight(self):
        self.assertEqual(yacht.score([6, 5, 4, 3, 2], yacht.LITTLE_STRAIGHT), 0)

    def test_no_pairs_but_not_a_big_straight(self):
        self.assertEqual(yacht.score([6, 5, 4, 3, 1], yacht.BIG_STRAIGHT), 0)

    def test_choice(self):
        self.assertEqual(yacht.score([3, 3, 5, 6, 6], yacht.CHOICE), 23)

    def test_yacht_as_choice(self):
        self.assertEqual(yacht.score([2, 2, 2, 2, 2], yacht.CHOICE), 10)

No, but

    elif category == 'FOUR_OF_A_KIND':
        dice = sorted(dice)
        if dice[0] == dice[3] or dice[1] == dice[4]:
            return dice[1] * 4 or 0

Does.

As does

case 'FOUR_OF_A_KIND' if dice[0] == dice[3] or dice[1] == dice[4]:
    return sorted(dice)[1] * 4

Or other variants where dice is sorted up front. However, sorting is harder with the lambda solution.

Yes, if you’re sorting the dice they will work. I don’t deny that.
(maybe I shouldn’t have interfered in this discussion, I was just wondering what you two were talking about and wanted to make it easier for others like me to figure that out.)

I actually don’t care much. If you want to propose a change to the approaches, go ahead and submit a PR. I probably won’t get to reviewing it until next week, but I can go over it then.

It wasn’t interfering. :smile: And you are welcome in any (and all!) discussions. Seriously.

I just don’t want this dragging on over details - or over extra test cases. This exercise is a bit problematic to begin with.

The critique is valid, so a PR can be made to adjust.

1 Like

Or other variants where dice is sorted up front. However, sorting is harder with the lambda solution.

Neither of them do.

Try running my test file locally against your solution.