-
-
Notifications
You must be signed in to change notification settings - Fork 555
ETL: Modified canonical description to ease test suite generation #1485
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,77 +1,76 @@ | ||
{ | ||
"exercise": "etl", | ||
"version": "1.0.0", | ||
"version": "1.0.1", | ||
"comments": [ | ||
"Transforms a set of scrabble data previously indexed by the", | ||
"tile score to a set of data indexed by the tile letter", | ||
"Note: The expected input data for these tests should have", | ||
"integer keys (not stringified numbers as shown in the JSON below", | ||
"Unless the language prohibits that, please implement these tests", | ||
"such that keys are integers. e.g. in JavaScript, it might look ", | ||
"like `transform( { 1: ['A'] } );`" | ||
], | ||
"cases": [ | ||
{ | ||
"comments": [ | ||
"Note: The expected input data for these tests should have", | ||
"integer keys (not stringified numbers as shown in the JSON below", | ||
"Unless the language prohibits that, please implement these tests", | ||
"such that keys are integers. e.g. in JavaScript, it might look ", | ||
"like `transform( { 1: ['A'] } );`" | ||
], | ||
"description": "transforms the a set of scrabble data previously indexed by the tile score to a set of data indexed by the tile letter", | ||
"cases": [ | ||
{ | ||
"description": "a single letter", | ||
"property": "transform", | ||
"input": { | ||
"1": ["A"] | ||
}, | ||
"expected": { | ||
"a": 1 | ||
} | ||
}, | ||
{ | ||
"description": "single score with multiple letters", | ||
"property": "transform", | ||
"input": { | ||
"1": ["A", "E", "I", "O", "U"] | ||
}, | ||
"expected": { | ||
"a": 1, | ||
"e": 1, | ||
"i": 1, | ||
"o": 1, | ||
"u": 1 | ||
} | ||
}, | ||
{ | ||
"description": "multiple scores with multiple letters", | ||
"property": "transform", | ||
"input": { | ||
"1": ["A", "E"], | ||
"2": ["D", "G"] | ||
}, | ||
"expected": { | ||
"a": 1, | ||
"d": 2, | ||
"e": 1, | ||
"g": 2 | ||
} | ||
}, | ||
{ | ||
"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"] | ||
}, | ||
"expected": { | ||
"a": 1, "b": 3, "c": 3, "d": 2, "e": 1, | ||
"f": 4, "g": 2, "h": 4, "i": 1, "j": 8, | ||
"k": 5, "l": 1, "m": 3, "n": 1, "o": 1, | ||
"p": 3, "q": 10, "r": 1, "s": 1, "t": 1, | ||
"u": 1, "v": 4, "w": 4, "x": 8, "y": 4, | ||
"z": 10 | ||
} | ||
} | ||
] | ||
"description": "transforms a single letter", | ||
"property": "transform", | ||
"input": { | ||
"data" : {"1": ["A"]} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How does inserting "data" between "input" and the actual data make things easier? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Strictly by definition (and if automatically generating the tests), each input translates as a new parameter to a function - so the previous data is confusing - you end up with a function that takes : Transform(input1: x, input2: y, input 3: z etc)... which is not the intent of the exercise. Contrast this with other exercises where the function genuinely has these parameters - eg calculateArea(width:5, length: 10). The last test for this etl exercise drives it home (with so many inputs). It’s inconsistent with other exercises as previously defined. And if you look at how other languages have manually translated it - they all pass a single dictionary as the input. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ErikSchierboom , I'm wondering if you have any comment on this one too - as presumably you might have hit this inconsistency when generating exercises from the spec too? I notice that Etl in C# also calls a single function and passes a single dictionary with "1" and "2" as keys. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @macta You are correct that in principle, it would be slightly easier to have the inputs grouped into a single parameter. |
||
}, | ||
"expected": { | ||
"a": 1 | ||
} | ||
}, | ||
{ | ||
"description": "transform a single score with multiple letters", | ||
"property": "transform", | ||
"input": { | ||
"data" : {"1": ["A", "E", "I", "O", "U"]} | ||
}, | ||
"expected": { | ||
"a": 1, | ||
"e": 1, | ||
"i": 1, | ||
"o": 1, | ||
"u": 1 | ||
} | ||
}, | ||
{ | ||
"description": "transform multiple scores with multiple letters", | ||
"property": "transform", | ||
"input": { | ||
"data" : { | ||
"1": ["A", "E"], | ||
"2": ["D", "G"]} | ||
}, | ||
"expected": { | ||
"a": 1, | ||
"d": 2, | ||
"e": 1, | ||
"g": 2 | ||
} | ||
}, | ||
{ | ||
"description": "transforms multiple scores with differing numbers of letters", | ||
"property": "transform", | ||
"input": { | ||
"data" : { | ||
"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"]} | ||
}, | ||
"expected": { | ||
"a": 1, "b": 3, "c": 3, "d": 2, "e": 1, | ||
"f": 4, "g": 2, "h": 4, "i": 1, "j": 8, | ||
"k": 5, "l": 1, "m": 3, "n": 1, "o": 1, | ||
"p": 3, "q": 10, "r": 1, "s": 1, "t": 1, | ||
"u": 1, "v": 4, "w": 4, "x": 8, "y": 4, | ||
"z": 10 | ||
} | ||
} | ||
] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not the correct next version. Changing the input like this is not backwards compatible, so should probably be "2.0.0".