ETL: Modified canonical description to ease test suite generation #1485
Conversation
|
Also - the use of a nested case was redundant, they were essentially all top level. |
rpottsoh
changed the title
| "description": "transforms a single letter", | ||
| "property": "transform", | ||
| "input": { | ||
| "data" : {"1": ["A"]} |
There was a problem hiding this comment.
How does inserting "data" between "input" and the actual data make things easier?
There was a problem hiding this comment.
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.
@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.
@macta You are correct that in principle, it would be slightly easier to have the inputs grouped into a single parameter.
The "Also" there implies to me that this PR should be two commits, one for removing the extra nesting and one for adding |
| @@ -1,77 +1,76 @@ | |||
| { | |||
| "exercise": "etl", | |||
| "version": "1.0.0", | |||
| "version": "1.0.1", | |||
There was a problem hiding this comment.
This is not the correct next version. Changing the input like this is not backwards compatible, so should probably be "2.0.0".
|
this was done in the linked PR |
Small change to the json spec that is easier for code generation, and clearly shows that you are calling the etl function with 1 parameter (which has "1"..."x" tile arrays)