transpose: Align 'test many lines' expected result with javascript 'many lines' test

[peerreynders]

See #199 (comment)

Core changes:

• Updated 'test many lines' test case to have an expected result identical to the one in the JavaScript track which represents the spirit of the transpose exercise.
• Provided example implementation capable of satisfying all the the tests including the updated one.

Opinionated Changes:

(1) Provided a reference implementation that relies on more rudimentary language primitives to satisfy the tests. The objective is not to provide some kind of a "gold standard" but instead to provide a specimen that together with "descriptive and meaningful names" can be easily understood by a beginning student while being easily improved upon by an intermediate student. This may not be desirable if the reference implementation is used as a mentoring aid.

The last example implementation (for me personally at least) was terse enough to require significant scratch refactoring to locate the root cause of the problem:

class Transpose {
  public static transpose(lines: string[]): string[] {
    const spreadLineChars: (t: string[], l: string , n: number) => string[] =
      (transposed, sourceLine, sourceIndex) => {
        const appendTargetLine: (c: string, l: number) => void =
          (char, targetIndex) => {
            transposed[targetIndex] =
              (targetIndex in transposed ?
               transposed[targetIndex] :
               ' '.repeat(sourceIndex)
              ) + char
          };

        sourceLine.split('').forEach(appendTargetLine);
        return transposed
      };

    return lines.reduce(spreadLineChars, [])
  }
}

What confused the issue was that Array.prototype.map was used for side effects (replaced above with forEach) and to some degree that reduce mutated the accumulator rather than recreating it with modifications (in the spirit of map and filter).

The revised reference implementation stays with mutation (which in this small scope is often judged to be easier to understand despite the hazards on a larger scale) and uses for loops to stay with the theme.

The reference implementation doesn't reflect my personal taste - I kind of landed on:

function transpose(lines: string[]): string[] {
  const length = lines.length;
  let maxHeight = 0;
  const mirrorPadSplit: (v: string[], i: number) => string[] =
    (_v, index) => {
      const mirrorLine = lines[length - index - 1];
      if (mirrorLine.length > maxHeight) maxHeight = mirrorLine.length;
      return mirrorLine.padEnd(maxHeight).split('');
    };
  const revSource = Array.from({ length }, mirrorPadSplit);

  const lineByColumn: (v: string, i: number) => string =
    (_v, columnIndex) => {
      const appendLine: (l: string, c: string[]) => string  =
        (line, chars) => columnIndex in chars ? line + chars[columnIndex] : line;

      return revSource.reduceRight(appendLine, '');
    };
  return Array.from({ length: maxHeight }, lineByColumn);
}

(2) Changed the transpose implementation from a static class method to a simple function in both the tests and reference implementation. Given what transpose is doing and how the tests use it a class seems to be an awkward representation of the capability - a function exported by the module seems more appropriate. To make this very clear the starter transpose.ts already includes a function implementation that passes the first test.

(3) Added the "jagged triangle" test. This small test case demonstrates the problem that the previous reference implementation had much more clearly than the final "test many lines" test case.

[SleeplessByte]

Quick question, do you know why a few of the package.json are changed from 100644 → 100755? We don't want package.json to be 755, I think?

[SleeplessByte]

Changes look sensical. Thank you for doing the work.

See inline comment and PR question.

[SleeplessByte]

Only request I have is to also push this addition to problem-specifications.

1. generate a uuid (google a generator, generate one, or use configlet uuid if you have configlet)
2. add it to .meta/tests.toml
3. PR it to this file.

I'm also in that repo, the process won't be lengthy to get it merged there.

[peerreynders]

A.) problem-specification pull request: exercism/problem-specifications#1748
In hindsight you specified a range - did you want to replace the "triangle" test with "jagged triangle"?
Because I simply added a new test at the end.

B.) added the uuid to the tests.toml

C.) "package.json are changed from 100644 → 100755"?
It's something that must have happened when I ran through the lint, test, sync, ci-check, ci scripts.

[SleeplessByte]

C) Yeah it was possibly wrong already 💯 . I fixed it :)

[SleeplessByte]

A) as you've done it right now looks perfect to me.

[SleeplessByte]

Thank you @peerreynders