Replace anchor tags with spans, and place the href attribute into the span

Question

I'm required to perform a slight conversion on a string which contains html.

The conversion should transform a elements such that they become span elements. The conversion should also place the href attribute from the original a element into the span.

I have what I think is a working solution, but the code feels overcomplicated. I had hoped this may be possible with relatively little code, so I was wondering if anyone could suggest ways to make this code neater.

Browser compatibility is not an issue, as this will be transpiled by babel.

Here's what I have:

const testHtml = `
  <p>This is a test paragraph</p>
  <a style="font-weight: bold;" href="http://www.google.co.uk">Google is here</a>
  <a style="font-weight: bold;" href="http://www.msn.co.uk">Msn is here</a>
  <h2>This is a heading</h2>
`

const splitByLinkEnds = html => {
  const split = html.split('</a>')
  const allButLast = split.slice(0, -1)
  const last = split.slice(-1)[0]

  return [
    ...allButLast.map(x => `${x}</a>`),
    last
  ]
}

const splitByLinks = html => {
  const split = html.split('<a');
  const [
    first,
    ...result
  ] = split

  const withReaddedOpeningTags = [
    first,
    ...result.map(x => `<a${x}`)
  ]

  return withReaddedOpeningTags
    .reduce((prev, curr) => [
      ...prev,
      curr.indexOf('<a') > -1 ?
        splitByLinkEnds(curr) : curr
    ], []).flat()
}

const transformLinkToTextOnly = linkHtml => {
  const linkHref = linkHtml
    .match('href="[^"]*"')[0]
    .replace('href="', '')
    .replace('"', '')

  const htmlWithSpan = linkHtml
    .replace('<a', '<span')
    .replace('</a>', '')

  return `${htmlWithSpan} (${linkHref})</span>`
}

const transformLinksToTextOnly = html =>
  splitByLinks(html)
    .map(x => x.indexOf('<a') > - 1 ?
      transformLinkToTextOnly(x) : x
    ).join('')

const result = transformLinksToTextOnly(testHtml)

document.body.innerHTML += testHtml
document.body.innerHTML += '<hr />'
document.body.innerHTML += result

---

Edit I should note that the solution ought to work in node, ideally. I appreciate that the fact I include dom manipulation in the question is quite misleading, I included this more to attempt to create an easy working example that to demonstrate how the code might actually be used.

A further note, the performance of this code is not particularly important. It's only for relatively small amounts of html and gets run once when the application loads.

Answer 1

One thing to consider is the whole working with HTML as string. In the answer I'll assume you work within the web-browser. The web-browser can parse the HTML for you, this way you'll only have to work with nodes.

Here is an example:

const testHtml = `
  <p>This is a test paragraph</p>
  <a style="font-weight: bold;" href="http://www.google.co.uk">Google is here</a>
  <a style="font-weight: bold;" href="http://www.msn.co.uk">Msn is here</a>
  <h2>This is a heading</h2>
`;

function transformLinksToTextOnly(htmlString) {
  const root     = document.createElement('root');
  root.innerHTML = htmlString;
  const anchors  = root.querySelectorAll('a');

  anchors.forEach(anchor => {
    const span = document.createElement('span');

    span.innerHTML = anchor.innerHTML;
    if (anchor.href) 
      span.innerHTML += ` (${anchor.href})`;

    anchor
      .getAttributeNames()
      .forEach(attrName => {
        const attrValue = anchor.getAttribute(attrName);
        span.setAttribute(attrName, attrValue);
      });

    anchor.parentNode.replaceChild(span, anchor);
  });

  return root.innerHTML;
}

const result = transformLinksToTextOnly(testHtml);

document.body.innerHTML += testHtml;
document.body.innerHTML += '<hr />';
document.body.innerHTML += result;

Answer 2

Highly inefficient

The code is highly inefficient, wasting CPU cycles and memory due to poor use of Javascript API's. It is also badly formatted and thus hard to read and follow and poor naming also made working out what you were doing difficult.

Code Points.

• Better naming alround. Example, you have two functions transformLinkToTextOnly and transformLinksToTextOnly Now I might be a bit blind but without spaces its not at all clear that these are two separate functions. If you consider the context of the functions and encapsulate them you can reduce the complex names to transformLink and transformLinks making the plural stand out, the rest is inferred due to context.
• Indent chained functions.
• Use semicolons to end lines. There are many arguments for and against using semicolons. Semicolons are required by the language, and are inserted automatically when javascript is parsed, however there are many edge cases that most people are unaware of and can catch you out. The rule of thumb is "Can you list all edge cases where semicolon insertion can cause problems?" if not use them. It is even more important that you are consistent. Even if you know all the rules, using semicolons in some places and not in others breaks the style rule "Be consistent".
• Avoid single use variable declarations.
• If you use terneries in multi expression lines, put them inside (...) for clarity. See example A
• Know your javascript. It is important that you are thoroughly familiar with the language you write in. You need to regularly review the JS reference and know the most concise and performant forms to achieve what you desire. Your code is very GC (garbage collection) unfriendly creating many arrays and thrashing memory and slowuing down the code, all of which can be avoided . See example B

Example A

  return withReaddedOpeningTags
    .reduce((prev, curr) => [
        ...prev,
      curr.indexOf('<a') > -1 ?
            splitByLinkEnds(curr) : curr
    ], []).flat()

Better syntax written as

return withReaddedOpeningTags
    .reduce((prev, curr) =>
        [...prev, (curr.indexOf('<a') > -1 ? splitByLinkEnds(curr) : curr)]
        , [])
    .flat();

As I am at this function I will point out that for every iteration you make a new copy of the array, add it as another array, and in the end you are forced to flatten the result to get what you want.

The same can be done with.

return withReaddedOpeningTags
    .map(item =>  item.indexOf('<a') > -1 ? splitByLinkEnds(item) : item);

or

return withReaddedOpeningTags
    .map(item => item.includes('<a') ? splitByLinkEnds(item) : item);

Example B

An example function splitByLinkEnds create 5 arrays

const splitByLinkEnds = html => {
    const split = html.split('</a>');       // array 1

    const allButLast = split.slice(0, -1);  // array 2
    const last = split.slice(-1)[0];        // creates array 3 an uses first item

    return [
        ...allButLast.map(x => `${x}</a>`),  // map creates an array 4 that is then 
                                             // copied to the return array 5
        last 
    ];
}    

It can be done using only one new array.

const splitEnds = markup => {
    const parts = markup.split("</a>");
    const end = parts.pop();
    parts.forEach((x, i) => parts[i] = x + "</a>");
    parts.push(end);
    return parts;
}

or as

const splitEnds = markup => {
    return markup.split("</a>").forEach((x, i, a) => a[i] = x + (i < a.length - 1 ? "</a>" : ""));
}

Rewrites

I have include 4 ways of achieving the same result. Javascript is very expressive so there is no single right way to do anything.

Rewrite using only strings and arrays

This method does not require the DOM and splits the markup at anchor boundaries creating an array of anchor descriptions which is then used to replace the anchors in the original markup.

function replaceAnchors(markup) {
    function findLinks(markup) {
        return markup.substring(markup.indexOf("<a") + 2).split("<a")
            .map(markup => ({
                markup : "<a" + markup.split("</a>")[0] + "</a>",
                anchor : markup.split(">")[0],
                href : markup.split("href=\"")[1].split("\"")[0],
                text : markup.split(">")[1].split("</a")[0],
            }));
    }        
    findLinks(markup).forEach(link => {
        markup = markup.replace(link.markup,`<span${link.anchor}>${link.text}(${link.href})</span>`);
    });
    return markup;
}

Second version does away with the intermediate array and does it in one pass.

function replaceAnchors(markup) {
    markup.substring(markup.indexOf("<a") + 2).split("<a")
        .forEach(a => {
            const href = a.split("href=\"")[1].split("\"")[0];
            const text = a.split(">")[1].split("</a")[0];

            markup = markup.replace(
                `<a${a.split("</a>")[0]}</a>`,
                `<span${a.split(">")[0]}>${text}(${href})</span>`
                );
        });
    return markup;
}

Rewrite with memory and performance in mind.

This uses named properties to determine what to do with each character in turn. If a match[current] is found then actions[current].add and actions[current].action are performed, each action sets up the next expected match and actions by setting the string current to the correct name. It is very fast and has a little room to go even faster. It is also very memory efficient

function replaceAnchors(markup) {
    const match = {
        get anchorStart() { return markup[idx] === "<" && markup[idx+1] === "a" },
        get hrefStart() { return markup.substring(idx,idx + 6) === "href=\"" },
        get hrefEnd() { return markup[idx] === "\"" },
        get tagClose() { return markup[idx]  === "<" && markup[idx+1]  === "/" && markup[idx+2]  === "a" },
    };
    const actions = {
        anchorStart: {
            add() { idx += 2; return "<span" },
            action() { current = "hrefStart" }
        },
        hrefStart: {
            add() { idx += 6; return "href=\""}, 
            action() { hrefStart = idx; current = "hrefEnd" }
        },
        hrefEnd: {
            add() { idx += 1; return "\""}, 
            action() { href = markup.substring(hrefStart, idx -1); current = "tagClose" }
        },
        tagClose: {
            add() { idx += 4; return "(" + href + ")</span>" },
            action() { current = "anchorStart" }
        },
    }
    var result = "", idx = 0, hrefStart, href, current = "anchorStart";
    while (idx < markup.length) {
        if (match[current]) {
            result += searchs[current].add();
            searchs[current].action();
        } else {
            result += markup[idx++]; 
        }
    }
    return result;
}

Rewrite using the DOM

This method uses the Markup parser to help locate anchor tags and then just replaces the start and end tag with span and the additional href

function replaceAnchors(markup) {
    const nodes = document.createElement("span");
    nodes.innerHTML = markup;
    for (const a of nodes.querySelectorAll("a")) {
        const anchor = a.outerHTML;
        markup = markup.replace(anchor, anchor
            .replace("</a>",`(${a.getAttribute("href")})</span>`)
            .replace("<a","<span")
        );

    }
    return markup;
}