1
\$\begingroup\$

I have written this function for creating complete HTML-elements including arbitrary attributes and inner HTML.

I want to keep and reuse it. Therefore I have tried to document the usage well.

I would be interested in hints concerning:

  • Could the code be improved? So that it is becomes more succinct and efficient?

  • Can one expect the function to work as intended even when it isn't used correctly?

  • Is the documentation understandable? Or does it have to be improved?

  • Is there a way to check if a valid HTML node name has be assigned as first parameter? Currently I'm only checking if the parameter has been assigned and is of type string.

/**
 * Returns an HTML-element of the specified node type,
 *  with the specified attributes and text.
 * 
 * @param { string } nodeName - E.g. 'div', 'li', 'p'
 * @param { string } attrs - The attributes of the 
 *  HTML-element. Each attribute has to be defined
 *  as <name>=<value>. Multiple attributes have to
 *  be separated with a blank between each pair.
 *  Example:
 *  'value=12 class=option-item data-test=123Demo'
 * @param { string } innerHtml - String for the
 *  innerHTML of the HTML element.
 *
 * @return { Object } - The created HTML element.
 * @throws { Object } - In case of error.
 *
 * Usage example:
 * wrap.appendChild( getHtmlElement(
			  'div', 
		          'class=spec data-shipping=express id=123',
		          'Your package will be shipped soon.' ));
 */
			
function getHtmlElement(nodeName, attrs, innerHtml) {
  var ret = null;

  if (!nodeName || typeof nodeName !== 'string') {
    throw { message: 'No nodeName-parameter assigned.' }
  }

  attrs = attrs || '';
  innerHtml = innerHtml || '';

  ret = document.createElement(nodeName);

  attrs = attrs.split(/\s/g);

  if (attrs[0]) {
    attrs.forEach(function(attr) {
      attr = attr.split(/=/);

      if (attr[1]) {
        ret.setAttribute(attr[0], attr[1]);
      }
    });
  }

  ret.innerHTML = innerHtml;		

  return ret;
}

// Usage-example:
document.body
        .appendChild(getHtmlElement(
                     'p', 
                    'class=inserted-paragraph data-info=paragraph-number-one',
                     'Lorem ipsum <i>dolor</i> sit amet, <b>consectetuer</b> ' +
                       'adipiscing elit. Aenean <u>commodo</u> ligula eget dolor.'));

\$\endgroup\$
3
  • 1
    \$\begingroup\$ Why not simply document.body.insertAdjacentHTML('beforeend', '<p class=....>.......</p>') on multiple lines for the same readability? \$\endgroup\$
    – woxxom
    Commented Oct 12, 2016 at 13:42
  • \$\begingroup\$ @wOxxOm Have just read about it. Awesome ! Thanks for the hint. And it seems to be faster too from what they write on MDN ... \$\endgroup\$ Commented Oct 13, 2016 at 6:01
  • \$\begingroup\$ ... and less invasive then writting into .innerHTML. \$\endgroup\$ Commented Oct 13, 2016 at 6:19

1 Answer 1

3
\$\begingroup\$

Some thoughts below:

  • Consider passing attrs as an object or object literal representing key:value pairs. This would eliminate need for more fragile string parsing approach you currently have.

You could replace all of this:

  attrs = attrs || '';
  innerHtml = innerHtml || '';

  ret = document.createElement(nodeName);

  attrs = attrs.split(/\s/g);

  if (attrs[0]) {
    attrs.forEach(function(attr) {
      attr = attr.split(/=/);

      if (attr[1]) {
        ret.setAttribute(attr[0], attr[1]);
      }
    });
  }

with this:

  attrs = attrs || {};
  innerHtml = innerHtml || '';

  ret = document.createElement(nodeName);

  for (attr in attrs) {
        ret.setAttribute(attr, attrs[attr]);
  }
  • Consider different name. "get", in typical programming thought often refers to retrieval of an existing item. I would think createHtmlElement or similar would be more appropriate.
  • I would strongly consider adding some additional validation around the input node name, to at a minimum make sure you have a non-zero length string.
  • After element creation, you could determine whether ret contains HTMLUnknownElement object, which would indicate that the browser did not recognize the node name value. This could be useful if you wanted to fail out at this point to not allow arbitrary tag creation. Perhaps whether or not to allow arbitrary (custom) tag creation could even be a flag passed as optional parameter.
  • Consider throwing known Error object rather than vanilla object literal of your own creation. You get a lot more utility from this Error object than from generic object. Check out the documentation at link below:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error

Example:

throw new Error('error message');
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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