6
\$\begingroup\$

The code below was inspired by this post in Code Review.
Here is how it was first intended by its author:

I have the following code that converts a string that looks like :

aaa-bbb|ccc-ddd|eee-fff

to two strings that look like

aaa|ccc|eeeandbbb|ddd|fff

Somebody already proposed an answer, and I found it could be more improved using reduce() and map() methods.
But after a few time I realized it could be made of more general use, so I hooked in the intent to have a code which both:

  • accept any main- and sub-separator (| and -) in the above example.
  • accept any number of main parts and sub-parts, the only (not checked) requirement being the number of sub-parts is the same through all parts.
  • is strictly written functional-programming style

Then I ended up with pretty different requirements than in the original post I cited, so rather than an answer I prefer to post it as a question, waiting for comments about:

  • is there a simpler strategy for doing that in functional style?
  • and even is it a good idea to use functional style, since it might be actually slower than with procedural code?

function reformat(string, mainSep, subSep) {
  return string.split(mainSep).map(item => item.split(subSep)).reduce(
    (result, part) => 
      part.map((str, index) => 
        `${result[index]}${mainSep}${str}`
      )
  ).join(subSep || ' ');
}

console.log(reformat('HEL-CAS|MAD-STO|XXX-YYY', '|', '-')); 
console.log(reformat('1:2:3_one:two:free_ONE:TWO:THREE_I:II:III', '_', ':'));
console.log(reformat('abcdefgh.ABCDEFGH', '.', ''));

\$\endgroup\$
1
\$\begingroup\$

To start, I must say that I wish I was writing JS like this in 2016. I have only recently been becoming acquainted with the newer features has to offer. After looking at the other answers to the post you mentioned I would have suggested they consider using destructuring assignment, but I doubt it would help for your code.

For readability it can be a great idea to use functional code, but yes performance can suffer because each iteration calls a function, which adds to the call stack.

This code makes great use of template literals and arrow functions. It also has consistent indentation.

The template literal may be considered excessive, since it only contains interpolated values. The string concatenation could be achieved with the + operator - instead of:

 part.map((str, index) => 
       `${result[index]}${mainSep}${str}`

It could merely be:

 part.map((str, index) => 
       result[index] + mainSep + str

Which would save a few characters but also should be faster. See this jsBin for a comparison, as well as this answer to Most efficient way to concatenate strings in JavaScript? which has a link to an updated JSPerf. The graphs don't appear to be loading in my browser but in MacOS FF 77 the concat appears to be fastest with the template literal 4% slower. In Chrome 83.04 the results were similar - template literals appeared to be 22% slower than appending.

I forgot that reduce() started with the 2nd element when no initialValue is supplied. While it may not be a huge improvement it does mean one less iteration...

| improve this answer | |
\$\endgroup\$
1
\$\begingroup\$

I think a functional style is a good choice. Provided you have the right sort of algorithmic complexity, I don't think you should worry about performance until it shows itself to be necessary.

That being said, I think there's room for improvement if you're aiming to be functional, or at least doing things in another way that has other benefits. An idea of functional programming is to write small functions which you then compose.

Something I don't like about your reformat function is that it joins the array at the end (and also less directly in the reduce), but the array is the better representation of what you want. Supposing I want to go back to the array, then I have to do a split, and know the right thing to split on, that's a bit sad. It's basically an example of a function doing too many things, its responsible for both parsing the string and using that to transform it, better to have a parser and a transformer, distinct functions.

It is better I feel to have a function which gets the array, and then your reformat function calls that and then does the joins.

Another thing is that I think you can make some things a bit more abstract so that you can reuse them. For instance your reduce is basically concerned with taking an array of arrays, and grouping up the ith term of each subarray. This is something that turns up very often and is called zip.

Of course what all answers provided so far are missing is handling of the the string not being well formed to do this operation, but I guess we are just assuming it is, which is sometimes right.

function zip(arrs) {
    const [firstArr, ...otherArrs] = arrs;
    return firstArr.map((v, i) => otherArrs.reduce((acc, arr) => [...acc, arr[i]], [v]));
}

function parseToArray(str, mainSep, subSep) {
    const doubleSplit = str.split(mainSep).map((s) => s.split(subSep));
    return zip(doubleSplit);
}

function reformat(str, mainSep, subSep) {
    return parseToArray(str, mainSep, subSep)
        .map((s) => s.join(mainSep))
        .join(subSep || " ");
}

console.log(reformat("HEL-CAS|MAD-STO|XXX-YYY", "|", "-"));
console.log(reformat("1:2:3_one:two:free_ONE:TWO:THREE_I:II:III", "_", ":"));
console.log(reformat("abcdefgh.ABCDEFGH", ".", ""));
| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.