Additional test case to 'Matching Brackets'

Programming Python
1

I suggest to add addtional test case to ‘Matching Brackets’ exersize. Something like this:

def test_new_pair_at_the_end(self) -> None:                                                                            
    self.assertEqual(is_paired("([]){}"), True)

The reason is that i managed to pass all test with wrong solution:

OPENERS=('[','{','(')
CLOSERS=(']','}',')')
def is_paired(input_string):
    input_string_filtered=''.join(symbol for symbol in input_string if (symbol in OPENERS or symbol in CLOSERS))
    all_paired=True
    while input_string_filtered!='':
        if input_string_filtered[0] in OPENERS and len(input_string_filtered)%2==0:
            end_checker= CLOSERS.index(input_string_filtered[-1]) if input_string_filtered[-1] in CLOSERS else 4
            neighbor_checker=CLOSERS.index(input_string_filtered[1]) if input_string_filtered[1] in CLOSERS else 4
            if OPENERS.index(input_string_filtered[0])==neighbor_checker:
                input_string_filtered=input_string_filtered[2:]
            elif OPENERS.index(input_string_filtered[0])==end_checker:
                input_string_filtered=input_string_filtered[1:-1]
            else:
                all_paired=False
                break
        else: 
            all_paired=False  
            break
    return all_paired
2

Care to explain in English succinctly what class of solutions that additional test would catch and why you think it would add value?

If there is a common/easy mistake to make that this catches, that can justify updating the exercise across tracks. Do you feel like this is a likely bug? Do you see it in any other community solutions?

If this is a very narrow bug that is unlikely to hit anyone else, it might be much harder to justify the change.

3

Most of the test cases of this exersize with ‘True’ result provide one pair of brackets with some combination of nested pairs inside it.
One exception is “test_several_paired_brackets” test. It provides two empty pairs of brackets.
I suggest to add test which will have at least two seperate pairs of brackets like in “test_several_paired_brackets” but with one of the pairs containing aditional nested pairs.

My reasoning:
The code above strips input string of everything but brackets. Then it starts while loop on modifeid string.
Inside while loop string gets stripped of every pair of brackets until it fails to find a pair for bracket in not empty string.
On every iteration of loop it is assumed that opener is the fist element of the string and closer is the second or the last.
This solution passes every current test but will not pass the one i suggested.

I didn’t find something akin to my solution among all other community solutions. Maybe I’m am unique kind of stupid.

Hope I wasn’t too verbosely.

4

If this test had existed, would your solution just be to add a third check, that the closer is second, last or ?

The fact that you need to check two locations already is a good indication that there may be a better approach. If you were happy to check two locations, would the test get you to add a third location or rethink your approach?

5

The thing is - I’ve got the idea of my solution from looking at tests. I’m pretty sure I would have stopped with my line of thought with suggested test in there.

I agree that needing to check two locations is an indication that there may be a better approach but it does not indicate that the approach in itself is wrong.

6

I would like to add to this conversation. I ran into the same situation when working on the PHP version of this problem and I feel that the suggested test case has merit. Take this code as an example

if (foo[1] === 0)
{
    return true;
}

This would be a fairly common pattern of brackets ([]){} and is not covered by the current testing suite. Just like the original poster, my first thought when approaching the problem was to check for two things:

  1. Does the first bracket match the last bracket?
  2. Does the first bracket match the second bracket?

I knew this approach would not pass the above solution and wanted to see if there were any other test cases I had failed to consider so I was surprised when all the tests passed.

I could have submitted the simplistic “correct” solution I had originally come up with but created the following test to use as a way to try and come up with a better, more generalized solution.

/**
 * uuid [ not sure how this is generated ]
 * @testdox Nested followed by pair
 */
public function testNestedThenPair(): void
{
    $this->assertTrue(brackets_match("if (foo[1] === 0){return true;}"));
}

I do believe that this test prompted me to rethink my original approach and I may not have done that if I had not noticed the missing test, assuming naively instead that I passed all the tests and had figured it out. I hope that hearing about my experience hitting this in another language will contribute meaningfully to the conversation.