Skip to content

Instantly share code, notes, and snippets.

@smolak
Last active July 26, 2019 15:35
Show Gist options
  • Star 1 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save smolak/7ff92ea5d821f2a1b11a0fb0a324d3f2 to your computer and use it in GitHub Desktop.
Save smolak/7ff92ea5d821f2a1b11a0fb0a324d3f2 to your computer and use it in GitHub Desktop.
// I have some testing framework in the scope
// Questions below:
// 1. Should the description be like it is here, in first `it`, or with `only` word added:
// "should return Fizz when the number is divisible only by 3"
// That would imply that the number is divisible by 3 only, which is technically not true, as it is divisible
// by 1 as well, and some numbers, e.g. 6, 9, and so on, are divisible by themselves.
it('should return Fizz when the number is divisible by 3', () => {
// 2. Is that name enough, having `any` in it, to be complete in what numbers are producing `fizz`.
// There is also a matter of adding `only` (similar to case #1) having `anyNumberDivisibleOnlyBy3`.
// 3. And if not, should there be e.g. three numbers divisible by 3 only and three assertions?
// The reason is that having just this one assertion and recreating the implementation from such test
// will most likely not give us the ususal result (division without change etc.), as the simplest
// code that would pass such test is `if (number === 3) return 'fizz';`
const anyNumberDivisibleBy3 = 3;
expect(fizzBuzz(anyNumberDivisibleBy3)).to.equal('Fizz');
});
// 4. Similar case as with 3
it('should return Buzz when the number is divisible by 5', () => {
// 5. Similar case like #2 and #3
const anyNumberDivisibleBy5 = 5;
expect(fizzBuzz(anyNumberDivisibleBy5)).to.equal('Buzz');
});
it('should return FizzBuzz when the number is divisible by 3 and 5', () => {
// 6. Similar case like #2 and #3
const anyNumberDivisibleBy3And5 = 15;
expect(fizzBuzz(anyNumberDivisibleBy3And5)).to.equal('FizzBuzz');
});
it('should return that number when it is not divisible by 3 nor 5', () => {
// 7. Similar case like #2 and #3
const anyNumberNotDivisibleBy3Nor5 = 1;
expect(fizzBuzz(anyNumberNotDivisibleBy3Nor5)).to.equal(anyNumberNotDivisibleBy3Nor5);
});
@DanilSuits
Copy link

I think Kevlin Henney's discussion of Leap Year Classification covers the same ground as your questions here.

  • The response should startWith "Fizz" when the input is divisible by three
  • The response should equal "Fizz" when the input is divisible by three, but not by five
  • The response should endWith "Buzz" when the input is divisible by five
  • The response should equal "Buzz" when the input is divisible by five, but not by three
  • The response should equal "FizzBuzz" when the input is divisible by the product of three and five.

I think you could argue that "startsWith" and "endsWith" could be better described by "contains", and capture that "FizzBuzz" is a union of the tokens concatenated in ascending order.

@jbrains
Copy link

jbrains commented Jul 24, 2019

This seems like a good exercise to use property-based testing, but if you wish to use only examples, then I would prefer to use a handful of examples to establish the pattern.

I like the variable names that you've chosen. I especially like the correct use of "nor", which native English speakers are steadily losing. :)

I find the test descriptions easy to understand, but I have one suggestion for possible improvement: "should return FizzBuzz when it would otherwise want to return both Fizz and Buzz". I'd never thought of this before, but it seems somehow better, because it says the same thing with more abstraction/fewer implementation details. I wonder what others think.

Similarly, we could describe the last example as "should return the number when it would otherwise return neither Fizz nor Buzz".

@jbrains
Copy link

jbrains commented Jul 24, 2019

I think you could argue that "startsWith" and "endsWith" could be better described by "contains", and capture that "FizzBuzz" is a union of the tokens concatenated in ascending order.

I like this, but I worry that it's too clever. It feels more like implementation details than describing the nature of the problem. Whatever you do, I value a clear, exhaustive case analysis matters more than any other part of the description.

I can't explain it, but "starts with" and "end with" feels like an unnecessarily indirect way of saying the same thing. Maybe that's merely a personal preference. Maybe organizing the behavior both ways works equally well. I say "Maybe" only because the "FizzBuzz" rule merely duplicates the "Fizz" and "Buzz" rules that say "start with" or "end with".

@DanilSuits
Copy link

Related: Carlo Pescio discusses overfitting, using FizzBuzz as an example: https://drive.google.com/file/d/0B59Tysg-nEQZOGhsU0U5QXo0Sjg/view?pref=2&pli=1

@smolak
Copy link
Author

smolak commented Jul 26, 2019

Thanks everyone for your input!

@jbrains
Copy link

jbrains commented Jul 26, 2019

@DanilSuits I read the "overfitting" section and skimmed the rest of the article, but I don't quite see the specific connexion to our discussion here that you intended. I agree with the advice to beware overfitting. I think I'm trying to avoid overfitting by reframing the mod 15 rule as a "both" rule: "When fizz and buzz both match, then output 'fizzbuzz'", abstracting away the reason to match fizz and the reason to match buzz. I'm trying to separate the classifying behavior (fizz, buzz, both, neither) from the output behavior (fizz -> "fizz", buzz -> "buzz", both -> "fizzbuzz", neither -> n), which has the advantage of nudging the implementation towards straightforward composition. This kind of design tends to help us create smaller, more-independent, more-recombinable pieces.

I can guess that this relates to "overfitting", but I'd like to know more about why you mentioned it. :) Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment