Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ETL: input elements define function parameter count #1536

Merged
merged 2 commits into from
Jul 1, 2019

Conversation

bencoman
Copy link
Contributor

This supersedes stale PR #1485. (It was easier to redo off HEAD than resolve conflict there)

Background - In practical terms for test generation, the the number of "input" elements is the number of parameters of the function-under-test. Considering the following as an example which specifies a function with three parameters as input...
all-your-base Canonical-data

   {
      "description": "single bit one to decimal",
      "property": "rebase",
      "input": {
          "inputBase": 2,
          "digits": [1],
          "outputBase": 10
       },

This is reflected by several language implementations:

all-your-base CSharp

Assert.Equal(expected, AllYourBase.Rebase(inputBase, digits, outputBase));    

all-your-base Ruby

converted = BaseConverter.convert(input_base, digits, output_base)

all-your-base ObjectiveC

NSArray<NSNumber *> *result = [ AllYourBase outputDigitsForInputBase:2 inputDigits:@[@1] outputBase:10 ];

By that pattern, the following...
etl canonical-data

        {
          "description": "multiple scores with multiple letters",
          "property": "transform",
          "input": {
            "1": ["A", "E"],
            "2": ["D", "G"]
          },

        {
          "description": "multiple scores with differing numbers of letters",
          "property": "transform",
          "input": {
             "1": ["A", "E", "I", "O", "U", "L", "N", "R", "S", "T"],
             "2": ["D", "G"],
             "3": ["B", "C", "M", "P"],
             "4": ["F", "H", "V", "W", "Y"],
             "5": ["K"],
             "8": ["J", "X"],
            "10": ["Q", "Z"]
          },

...would imply two separate functions, one with two parameters and another with seven parameters.
But actually "1","2","3","4","8","10" are part of the data, not part of the function definition.
The example language implementations correctly inferred the intent was to call a single-parameter function...

etl CSharp

    Assert.Equal(expected, Etl.Transform(input));

etl Ruby

    assert_equal expected, ETL.transform(old)

etl ObjectiveC

    NSDictionary<NSString *, NSNumber * > *results = [Etl transform: old];

It is better to be explicit that the transform function takes a single parameter of a legacy data map.
Thus the following canonical-data is proposed...

        {
          "description": "multiple scores with multiple letters",
          "property": "transform",
          "input": {
              "legacy": { 
                  "1": ["A", "E"],
                  "2": ["D", "G"] 
              }
          },

@bencoman
Copy link
Contributor Author

Hi @ErikSchierboom, I used CSharp as an example which I understand you are involved with. I'm curious if if/how your generator inferred the input data was for a function taking a single dictionary rather than a function taking several parameters.

@ErikSchierboom
Copy link
Member

Good question. There is nothing in the canonical data itself that encodes this, which is why there is special handling in this exercise's generator: https://github.com/exercism/csharp/blob/ed87d8c9818ea46a7db7856cc9a294ef6a2a10ae/generators/Exercises/Generators/Etl.cs#L26

Note: there are a few occasions that need special handling, and dictionary-related exercises are one of them.

@ErikSchierboom
Copy link
Member

Oh and by the way, I agree that one input parameter should be defined for the combination of data. 👍

Copy link
Member

@ErikSchierboom ErikSchierboom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Just one thing: the version number needs to be bumped.

@bencoman
Copy link
Contributor Author

AH yes, the version CI checks are working well. thx @ErikSchierboom
I guess this is "changing the type of one of the test data keys" ==> bump major.

@ErikSchierboom
Copy link
Member

I guess this is "changing the type of one of the test data keys" ==> bump major.

Yep!

Copy link
Member

@ErikSchierboom ErikSchierboom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work!

Copy link
Member

@rpottsoh rpottsoh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM too! Thanks for adding clarity.

Copy link
Member

@SleeplessByte SleeplessByte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 🚀 🚀

@SleeplessByte SleeplessByte merged commit 47a0970 into master Jul 1, 2019
@SleeplessByte SleeplessByte deleted the bencoman-etl-explicit-parameter-count branch July 1, 2019 11:14
thalesmg added a commit to thalesmg/haskell that referenced this pull request Oct 12, 2019
It seems that only a version bump is necessary.

Latest changes to this problem:

- exercism/problem-specifications#1536
- exercism/problem-specifications#1575
sshine pushed a commit to exercism/haskell that referenced this pull request Oct 14, 2019
It seems that only a version bump is necessary.

Latest changes to this problem:

- 1.0.0 to 1.0.1, descriptions too long: exercism/problem-specifications#1526
- 1.0.1 to 2.0.0, change input JSON format: exercism/problem-specifications#1536
- 2.0.0 to 2.0.1, capitalize Scrabble: exercism/problem-specifications#1575
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants