Dynamic UI generator

Question

I am developing a query builder which consumes OData Web API (won't go into details on how it works in the back end).

I have this EDM/Metadata which I am using to generate UI components. I created a JSFiddle as well. Is there any way I can improve this code?

function init() {
  $container = $("#container");
  $queryBuilderForm = $('<form autocomplete="off" id="queryBuilderForm"><div id="url" style=""></div><button id="btnUpdateUrl" type="button">Update Url</button></form>');
  $container.append($queryBuilderForm);
  $entityFilters = $("<div id='entityFilters'></div>");
  $queryBuilderForm.append($entityFilters);
}

function processData() {
  this.metadata = getMetadata();
  for (var e in this.metadata.dataServices.schema) {
    var schema = this.metadata.dataServices.schema[e];
    if (schema.entityType) {
      entities = schema.entityType;
      break;
    }
  }
}

function addExpand(id, options) {

  var generateOptions = constructOptions(options, true);
  $entityFilters.append($([
    '<div id="entityFilter_' + id + '" class="entity-filter">',
    '<select id="entities_' + id + '">', generateOptions, '</select>',
    '<div id="entities_' + id + '_cols"></div>',
    '</div>'
  ].join('')));

  function constructOptions(options, addEmptySelect) {
    var opt = [];
    if (addEmptySelect) {
      opt.push('<option value=-1> -- Select -- </option>');
    }
    $.each(options, function(index, e) {
      opt.push('<option value=' + e.name + '>' + e.name + '</option>');
    });
    return opt.join('');
  }
}

function addEvents(id) {
  var $entities = $('#entities_' + id);
  $('body').on('change', $entities, function(event) {
    var $e = $(event.target);
    var entityName = singularize($e.val());
    var entity = $.grep(entities, function(e) {
      return e.name == entityName;
    })[0];
    var columns = entity.property; // if index is not -1

    $e.parent().nextAll(".entity-filter").remove();
    var $entities_cols = $e.next().empty();
    $.each(columns, function(index, e) {
      var selectColumnHtmlId = 'selectcolumn_' + e.name;
      var $selectColumnLabel = $('<label />', {
        'for': selectColumnHtmlId,
        text: e.name
      });
      $selectColumnLabel.appendTo($entities_cols);
      $('<input />', {
        type: 'checkbox',
        id: selectColumnHtmlId,
        value: e.name
      }).prependTo($selectColumnLabel);
    });

    if ($(".entity-filter").length < config.expandLimit)
      addExpand($(".entity-filter").length, entity.navigationProperty);
  });


  $("#btnUpdateUrl").on("click", function() {
    updateUrl();
  })
}

function updateUrl() {
  var temp_ents = "";
  var temp_path = "";
  var temp_paths = [];
  var temp_cols = "&$select="; // if no expand remove "&"
  $("#entityFilters select").each(function(ei, ed) {
    if (ei === 1)
      temp_ents = temp_ents + "?$expand=";
    else {
      temp_ents = temp_ents + "/";
    }
    temp_ents = temp_ents + (ei === 0 ? singularize($(ed).val(), true) : $(ed).val());

    if (ei !== 0)
      temp_path = temp_path + (temp_path ? "/" : "") + $(ed).val();

    $(this).siblings().find("input:checked").each(function(ci, cd) {
      if (ei !== 0) {
        temp_cols = temp_cols + temp_path + "/"
      }
      temp_cols = temp_cols + $(this).val() + ",";
    });

    temp_paths.push(temp_path + (temp_path ? ", " : ""));
  });
  console.log(temp_ents + temp_cols);
  $("#url").text(temp_ents + temp_cols);
}

function singularize(s, reverse) {

  if (s.endsWith("1") || s.endsWith("2"))
    s = s.substring(0, s.length - 1);


  for (var i = 0; i < entityMapping.length; ++i) {
    var e = entityMapping[i];
    if (reverse) {
      if (e[0] === s)
        return e[1];
    } else {
      if (e[1] === s)
        return e[0];
    }
  }
  return s;
}

var config = {
  expandLimit: 3,
};
var entityMapping = [
  ['Address', 'Addresses'],
  ['Category', 'Categories']
];

var $container;
var $queryBuilderForm;
var $entityFilters;
var entities;

init();
processData();
addExpand(1, entities);
addEvents(1);

.entity-filter {
  min-height: 200px;
  max-width: 200px;
  float: left;
}

.entity-filter label {
  display: block
}

.entity-filter>div {
  max-height: 400px;
  overflow-y: auto;
}

<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.9.1/jquery.min.js"></script>
<div id="container"></div>

Edit

I had to remove metadata from this question as it was too much, it is still in JSFiddle added with full working example. Thanks

Answer 1

Naming Things

One, two, very-few letters long variable names are a bad practice. Even the things like indices can and should be given a real name. If something is called an i, or e, or ie, it should be treated as if the thing does not have a reason to exist. If a variable or a parameter exists only because it's enforced by a library/language (e.g. $.each will give us an index and an object args), it is possible to explicitly show which of them are unused by naming them _. This is a common practice in functional programming.

A very nasty side effect of names like i, e, ci, cd is the added code complexity which appears out of nowhere. The code is not self-explanatory anymore, and dealing with it requires constant on-the-fly translation, which in turn makes the reader think about unnecessary details rather then the intent of the code.

This is even applicable to the innocent looking id variable/parameter, because without looking around nobody can tell what does the id belong to...

DOM and JavaScript

Depending on the context, it may or may not be a good idea to manipulate HTML DOM directly via JavaScript/JQuery. Say, in modern single page application frameworks (like Angular, and similar) it's considered a bad practice to deal with DOM in the way it's done in the sample code. Even with these frameworks, there are exceptions of course, like the cases where one builds a widget library.

Reduce Mutable Things

Try to declare all your variables as const when possible. Otherwise, make them be let. Practice shows that the side effects and nuances of var are in the best case not obvious to the most of JavaScript developers, and in the worst case -- very confusing.

Terse Code

Assuming it's okay to use relatively fresh features of ECMAScript 6, certain things may be written more compactly without losing readability. Syntax constructions like "fat arrow" (=>), spread operator (...), string interpolation, etc. are extremely powerful. The fat arrow may be very effectively combined with $.each/.foreach, .map, .reduce, .filter, and many many other functions that can accept lambdas.

Here are a few examples of slightly improved function. Notice that even this code may be further polished.

processData

function processData() {
  return getMetadata() // Notice that we return a value here -- caller has to save the result into a variable
    .dataServices
    .schema
    .find(schema => schema.entityType != null)
    .entityType;
}

Now the invocation changes to var entities = processData();

addExpand

function addExpand(id, options) {
  const generateOptions = constructOptions(options);
  $entityFilters.append($([
    `<div id="entityFilter_${id}" class="entity-filter">`,
    `<select id="entities_${id}">${generateOptions}</select>`,
    `<div id="entities_${id}_cols"></div>`,
    `</div>`
  ].join('')));

  function constructOptions(options) {
    return [
      `<option value=-1> -- Select -- </option>`,
      ...$.map(options, option => `<option value=${option.name}>${option.name}</option>`)
    ]
    .join('');
  }
}

Consider passing $entityFilters into this function as a parameter. It will improve the code by not having a direct mutative access to global state which is a known source of bugs.

addEvents

function addEvents(id) {
  const $entities = $(`#entities_${id}`);
  $('body').on('change', $entities, event => {
    const $eventTarget = $(event.target);
    const entityName = singularize($eventTarget.val());
    const entity = $.grep(entities, candidate => candidate.name == entityName)[0];
    const columns = entity.property; // if index is not -1

    $eventTarget.parent().nextAll(".entity-filter").remove();
    const $entities_cols = $eventTarget.next().empty();
    $.each(columns, (index, column) => {
      const selectColumnHtmlId = `selectcolumn_${column.name}`;
      const $selectColumnLabel = $('<label />', { 'for': selectColumnHtmlId, text: e.name });
      $selectColumnLabel.appendTo($entities_cols);
      $('<input />', { type: 'checkbox', id: selectColumnHtmlId, value: column.name })
        .prependTo($selectColumnLabel);
    });

    if ($(".entity-filter").length < config.expandLimit)
      addExpand($(".entity-filter").length, entity.navigationProperty);
  });

  $("#btnUpdateUrl").on("click", () => updateUrl());
}

updateUrl -- excerpt

+= and ternary (?:) operator can sometimes be efficiently combined too, but this kind of changes is really putting many developers on the fence. I'm trying to stop myself with ternaries, as soon as I start thinking for too long whether it's readable or not...

$("#entityFilters select")
  .each((index, optionItem) => {
    temp_ents += index === 1 ? "?$expand=" : "/";
    temp_ents += index === 0 ? singularize($(optionItem).val(), true) : $(optionItem).val();