Java: binary-search: Test with List<String> use unsorted list

[hugueschabot]

The current test suite for binary-search contains two tests with a list of String.

• findsAValueInTheMiddleOfAnArray
• identifiesThatAValueIsNotIncludedInTheArray

The two tests use this list of String, which is not sorted alphabetically.

        List<String> sortedList = Collections.unmodifiableList(
                Arrays.asList("1", "3", "4", "6", "8", "9", "11")
        );

Based on the default implementation of Comparable<String> provided by String, the correctly sorted list is Arrays.asList("1", "11", "3", "4", "6", "8", "9").

A correct implementation of the binary search will provide the expected result with either the actual list and the correctly sorted one.

[hugueschabot]

Using a List<String>sorted alphabetically make the tests become different from the canonical data. For instance, in findsAValueInTheMiddleOfAnArray the value "6" is no longer in the middle in the array.

Maybe we should only use numeric types (subclass of Number) for canonical tests and add some tests for other types with a non-numeric sort (like String)?

[FridaTveit]

That's a good point @hugueschabot, thanks for raising it! 🙂 We mainly use strings and chars in these tests so that users can become familiar with generics. But you're right, the way the tests work now might be a bit strange and confusing for users. So I think we have a few options:

• Remove generics from these tests and just use Integer. This has the advantage of being an easy change and will make us most consistent with the canonical data. However, it means users can't use this exercise to become familiar with generics.
• As @hugueschabot suggested, use only numeric types for the canonical data tests and add additional tests for other types. This means we can keep generics but it will mean not being consistent with the canonical data. That can make these tests harder to maintain.

What do you think @hugueschabot, @sjwarner-bp, @exercism/java? Do you have any other suggestions? 🙂

[hugueschabot]

The canonical data for all seven exercises tagged with the generics topic uses integers only or strings only (i.e. they don't requires a generic implementation). However, I think the Java track needs to include exercises that require the usage of generics and ideally one with bounded type parameters. So, if we stick to canonical data for binary search the only one exercise left that requires bounded type parameter is binary-search-tree.

Happily, the canonical data from binary-search-tree uses single digit integers and actually are implemented in a test suite with multiple types.

Therefore I think we can use only List<Integer> in the binary-search test suite. After completion of this exercise, the binary-search-tree exercise is unlocked so the student can learn about generics.

[FridaTveit]

That sounds sensible. I'm happy to remove generics from this exercise 🙂 What do you think @sjwarner-bp?

[dpolivaev]

I would replace 11 by 91. So the strings and the numbers would have the same order after sorting.

[sjwarner-bp]

Oops, missed this one @FridaTveit ! I think that removing generics is a sensible and simple way forward over here :slightly_smiling_face:

I am less keen on editing values in individual tests of the suite because we'd move further away from any canonical-data, and it might give users a false understanding of how certain methods work :slightly_smiling_face:

[hugueschabot]

If we change the tests as proposed in #1512 I think we should warn the mentors accordingly. Maybe with a mention in the mentoring notes. What do you think @FridaTveit , @sjwarner-bp .

[FridaTveit]

Sure 🙂 I don't know much about how the mentoring notes work or who maintains them. There doesn't appear to be any for binary-search?

[dpolivaev]

I think that information about the changes can go over slack because it is only relevant within a short period of time after the change. Sure one could create a mentoring note too and remove it after about a month.

[FridaTveit]

Sure, sounds sensible 🙂 I'm happy to message the track-java channel when the PR to fix this is merged 🙂

[hugueschabot]

Good point @dpolivaev and @FridaTveit . Slack does seem to be used more than the mentoring notes.

I will create the mentoring note for binary-search in the following days.

[dpolivaev]

Actually, in my dream, when I review a solution I would like to see a list of all other solutions of the same exercise sorted by time in descending order, so that I can learn from the solutions and discussions already happened.

Further, in my dream, each exercise could have its own thread either on slack or somewhere else so that mentors can easier talk about it.

[FridaTveit]

PR has been merged and I've posted in track-java 🙂 Thanks @hugueschabot! 🙂

@dpolivaev If you have suggestions for changes to the exercism website you can raise an issue on https://github.com/exercism/exercism as that's not really something we can change here on this track 🙂 For changes to slack practices, just posting on slack for example in the track-java channel might be the best way to go about that? 🙂

[dpolivaev]

@FridaTveit I opened https://github.com/exercism/exercism/issues/4364