Add a test for the edge case where the value is an empty string, two-fer

During a mentoring session on the two-fer exercise, I thought it would be good to add an edge case when the string value is empty. I created a PR with that small test. Overall, the exercise is very simple. I’m leaving this topic for consideration by the community. Feel free to close this thread if you don’t find it relevant.

Why should an empty string be replaced with the default “you”? What about a string containing only a space? Multiple whitespace?

1 Like

There’s also some considerations here worth discussing: Suggesting Exercise Improvements | Exercism's Docs

1 Like

@IsaacG

Well, the cases you mention are good edge cases that I hadn’t considered because it’s something that happens in real life, a user sending an empty space, passing the verification filters. Having those cases covered at the code level is always good. I would recommend adding those mentioned tests to the test runner; with that, I think it would be quite complete.

But I also understand that it can affect many who have already solved the exercise. It’s my first time contributing here. If it’s a change that requires more work than it contributes, it’s better to close the discussion. If you think that having tests that cover more cases could contribute, that’s fine too.

Also, adding more tests could make the exercise more complicated, and I don’t think it’s convenient either. I believe the main problem with adding more tests is increasing the complexity of the solution. Perhaps I didn’t think it through so well at the time, from that point of view.

I thought the exercise also said “If a name is given” to use the name. If the given name is an empty string, then should that not be used?

  xit('empty name given', () => {
    const expected = 'One for you, one for me.'
    expect(twoFer('')).toEqual(expected)
  })
})

The test would be incorrect, since it was not 'One for , one for me.'

1 Like

If we’re ignoring invalid inputs, are any words which are not names invalid, too? What is and isn’t accepted can quickly become very complex and arbitrary.

1 Like

@IsaacG My goal is not to validate all types of arbitrary values. The idea of adding tests in cases of empty values is to prevent outcomes that do not meet the exercise description, as indicated by @kotp

‘One for , one for me.’

Should be

‘One for you, one for me.’

But you do know their name, you asked, they gave '' and so you would use that. Since we are not the expert of their name, and we asked, we should then use that as their name. It matches the exercise description strictly and precisely.

In other words, I indicated no such thing as a “do not meet the exercise description” since I know that it does.

1 Like

@kotp @IsaacG

Thank you guys, you’re both right in your points. Let’s consider this topic closed. Thank you very much for the help and the time in this discussion

2 Likes