HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Editing HTML based on instructions in a data structure

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
instructionseditingstructurebaseddatahtml

Problem

My getExtend() function, which will be included via require.js, has two inputs and one output:



  • HTML file content



  • config object which determine how to update the HTML, for example to create new script (as the first object in the array


extendedHtmlObject ) or update existing script content such as
attributes values)

  • the output should be the extended HTML with all the modifications.




Here is an example config object.
Please notice that that in the first element of the array (there is scenario which the next attribute is not provided which say to put the new script at the bottom) the next said put the script tag after script that have ID "ui-boot"

var extendedHtmlObject = [{
                    type: 'script',
                    action: 'new',
                    content: 'console.log(‘hello world’);',
                    next: "ui-boot"
                }, {
                    type: 'script',
                    id: "ui-boot",
                    action: 'upd',
                    innerElem: [{
                        type: 'attr',
                        id: 'data--ui-comersion',
                        content: '1.17'
                    }, {
                        type: 'attr',
                        id: 'src',
                        content: '/test/test2/-co.js'
                    }]
                }


This is the main function:

getExtend: function(htmlContent, extendedHtmlObject) {
    var self = this;
    if (extendedHtmlObject) {
        extendedHtmlObject.forEach(function(configs) {
            switch (configs.type) {
                case 'script':
                    htmlContent = self._handleScriptElement(htmlContent, configs);
                    break;
                case 'head':
                    break;
            }
        });
    }
    return htmlContent;
},


This method determines if I need to create a new script or update existing script attributes according to the input object

```
_handleScriptEl

Solution

Personal preference, but I'd suggest 2-space indentation. You'll most likely nest a lot of stuff in JS, and using 4-space indentation can easily throw you over to the right side of your screen.

Another approach to reducing indentation overhead is to "return early" or offload the functionality to a separate function and use a ternary operation. This is especially when you have single if blocks with no else.

function foo(){
  var something = ''
  if(condition){
    something = doSomething();
  }
  return something;
}

// is the same as returning early:

function foo(){
  var something = ''
  if(!condition) return something;
  something = doSomething();
  return something;
}

// is the same as using a ternary:

function foo(){
  return condition ? doSomething() : '';
}



This is the input: please notice that that in the firstobj of the array (there is scenario which the next attribute is not provided which say to put the new script at the bottom) the next said put the script tag after script that have ID "ui-boot"

When you have to explain your code, that's a sign that you have done something bad. In this case, you had to explain what next in the first element is, and id on the second element.

Being consistent in your objects as well as good naming schemes are a good start:

var extendedHtmlObject = [{
  action: 'new',
  type: 'script',
  properties: null,
  content: 'console.log(‘hello world’);'
  position: {
    anchor: '#ui-boot',
    placement: 'after'
  }
}, {
  action: 'update',
  type: 'script',
  properties: {
    id: 'ui-boot',
    src: '/test/test2/-co.js',
    'data--ui-comersion': '1.17',
  },
  content: '',
  position: null
}];


While there are legitimate use cases for a switch, it's usually a sign that indicates that some function is trying to do too many things. Also, switch can bloat easily and forgetting break is a danger waiting to happen. Consider using key-value pairs instead:

function actions(key, args){
  switch(key){ 
    case: 'foo':
      return doFoo(args);
      break;
    case: 'bar':
      return doBar(args);
      break;
    case: 'baz':
      return doBaz(args);
      break
  }
}

var value = actions(keyargs);

// is the same as

var actions = {
  foo: function doFoo(){...},
  bar: function doBar(){...},
  baz: function doBaz(){...}
};

var value = actions[key](args);



could I maybe improve it with JS prototype?

If this is a singleton module, it doesn't make sense to use prototypes. Prototypes make sense when you create multiple instances that "inherit" the same methods. If you only create one instance, then it doesn't make sense to use a mechanism that was meant for building multiple instances of something.

Code Snippets

function foo(){
  var something = ''
  if(condition){
    something = doSomething();
  }
  return something;
}

// is the same as returning early:

function foo(){
  var something = ''
  if(!condition) return something;
  something = doSomething();
  return something;
}

// is the same as using a ternary:

function foo(){
  return condition ? doSomething() : '';
}
var extendedHtmlObject = [{
  action: 'new',
  type: 'script',
  properties: null,
  content: 'console.log(‘hello world’);'
  position: {
    anchor: '#ui-boot',
    placement: 'after'
  }
}, {
  action: 'update',
  type: 'script',
  properties: {
    id: 'ui-boot',
    src: '/test/test2/-co.js',
    'data--ui-comersion': '1.17',
  },
  content: '',
  position: null
}];
function actions(key, args){
  switch(key){ 
    case: 'foo':
      return doFoo(args);
      break;
    case: 'bar':
      return doBar(args);
      break;
    case: 'baz':
      return doBaz(args);
      break
  }
}

var value = actions(keyargs);

// is the same as

var actions = {
  foo: function doFoo(){...},
  bar: function doBar(){...},
  baz: function doBaz(){...}
};

var value = actions[key](args);

Context

StackExchange Code Review Q#125134, answer score: 3

Revisions (0)

No revisions yet.