kotlin icon indicating copy to clipboard operation
kotlin copied to clipboard

Issue: Largest Series Product

Open blueglyph opened this issue 5 years ago • 2 comments

In this exercise, there are two tests which don't seem to be correct:

    @Test
    fun reports1ForEmptyStringAndEmptyProduct() {
        assertEquals(1, Series("").getLargestProduct(0))
    }

    
    @Test
    fun reports1ForNonEmptyStringAndEmptyProduct() {
        assertEquals(1, Series("123").getLargestProduct(0))
    }

The requirement is: Given a string of digits, calculate the largest product for a contiguous substring of digits of length n.

Is it logical to return the value 1 when the arguments don't allow to find a solution? It looks like those two tests should return an IllegalArgumentException:

  • first case: contradicts Given a string of digits
  • second case: contradicts substring of digits of length n

If this is indeed what the exercise should be, then it should be specified explicitly IMHO.

blueglyph avatar Jun 10 '19 16:06 blueglyph

Thanks @blueglyph for reporting this! We will take a look.

dector avatar Nov 11 '19 21:11 dector

Quoting from problem-specification/exercises/largest-series-product/canonical-data.json:

    {
      "comments": [
        "There may be some confusion about whether this should be 1 or error.",
        "The reasoning for it being 1 is this:",
        "There is one 0-character string contained in the empty string.",
        "That's the empty string itself.",
        "The empty product is 1 (the identity for multiplication).",
        "Therefore LSP('', 0) is 1.",
        "It's NOT the case that LSP('', 0) takes max of an empty list.",
        "So there is no error.",
        "Compare against LSP('123', 4):",
        "There are zero 4-character strings in '123'.",
        "So LSP('123', 4) really DOES take the max of an empty list.",
        "So LSP('123', 4) errors and LSP('', 0) does NOT."
      ],
      "description": "reports 1 for empty string and empty product (0 span)",
      "property": "largestProduct",
      "input": {
        "digits": "",
        "span": 0
      },
      "expected": 1
    },
    {
      "comments": [
        "As above, there is one 0-character string in '123'.",
        "So again no error. It's the empty product, 1."
      ],
      "description": "reports 1 for nonempty string and empty product (0 span)",
      "property": "largestProduct",
      "input": {
        "digits": "123",
        "span": 0
      },
      "expected": 1
    },

So the tests seem to be correct. Should we add a comment to the test file or would that be too much of a "spoiler" to the solution?

lathspell avatar Jan 22 '20 20:01 lathspell