Extracting distinct HTML list items into a JavaScript array

Question

This code is made to generate a list of unique elements of the array made of the other two arrays. The fact that we need two lists to convert in two different array is part of the task. This code handles with the task, but there's a lot of global variables. As far as I know, it is not good practice. Tell me, please, how can I improve it?

<body id="body">
<div>
  <ul id="ul1">
    <li>Любовь</li>
    <li>Надежда</li>
    <li>Вера</li>
  </ul>
</div>
<div>
  <ul id="ul2">
    <li>София</li>
    <li>Надежда</li>
    <li>Вера</li>
  </ul>
</div>
<button id="makeNewArray">Создать новый массив</button>
<script type="text/javascript">
  function init() {
    var btn = document.getElementById('makeNewArray'),
        arrLi1 = ul1.getElementsByTagName('li'),
        arrLi2 = ul2.getElementsByTagName('li'),
        arr1 = [],
        arr2 = [],
        result = [];
    for (var i = 0; i < arrLi1.length; i++) {
      arr1.push(arrLi1[i].innerHTML);
    }
    for (var j = 0; j < arrLi2.length; j++) {
      arr2.push(arrLi2[j].innerHTML);
    }
    commonArr = arr1.concat(arr2);
    nextInput:
      for (var k = 0; k < commonArr.length; k++) {
        var str = commonArr[k];
        for (var l = 0; l < result.length; l++) {
        if (result[l] == str) continue nextInput;
        }
        result.push(str);
      }
    btn.onclick = function() {
      var div = document.createElement('div');
      div.innerHTML = result;
      body.appendChild(div);
    };
  };
  window.onload = init;
</script>
  </body>

You don't have any global variables, they all local in init function. — jcubic, Dec 24 '15 at 11:25
Notice, that all the variables are used in init, you can declare commonArray and str in that function too. Unless they are used in some other script too. A common way is to enclose all the script within a single IIFE, that way you wouldn't have to use globals at all. — Teemu, Dec 24 '15 at 11:32

cFreed · Accepted Answer · 2015-12-27 16:25:34Z

As already noticed by previous comments, don't mind about global variables if they're declared by var inside of a function.
And you can do the same for commonArray and str, unless they're used elsewhere (obviously, I don't think so).

In the other hand, you can dramatically reduce your code!

First because you don't have to wait for onload event, since your <script> is located at the end of the <body>.
So you may replace this structure:

  function init() {
    var btn = document.getElementById('makeNewArray'),
    // ...computing result...
    btn.onclick = function() {
      var div = document.createElement('div');
      div.innerHTML = result;
      body.appendChild(div);
    };
  };
  window.onload = init;

by this one:

  document.getElementById('makeNewArray').onclick = function() {
    // ...computing result...
    var div = document.createElement('div');
    div.innerHTML = result;
    body.appendChild(div);
  };

Then in the computing result part, you can also work in a more light way, using less variables and less statements. This achieved essentially by not creating temporary containers, and directly populating (or not) the final result while iterating the source <ul>s. Looks like this:

<body id="body">
<div>
  <ul id="ul1">
    <li>Любовь</li>
    <li>Надежда</li>
    <li>Вера</li>
  </ul>
</div>
<div>
  <ul id="ul2">
    <li>София</li>
    <li>Надежда</li>
    <li>Вера</li>
  </ul>
</div>
<button id="makeNewArray">Создать новый массив</button>
<script type="text/javascript">
document.getElementById('makeNewArray').onclick = function() {
  var div = document.createElement('div'),
      sources = [ul1, ul2],
      result = [];
  for (var i = 0, n = sources.length; i < n; i++) {
    var items = sources[i].getElementsByTagName('li');
    for (var j = 0, m = items.length; j < m; j++) {
      var str = items[j].innerHTML;
      if (result.indexOf(str) == -1) {
        result.push(str);
      }
    }
  }
  div.innerHTML = result;
  body.appendChild(div);
};
</script>
</body>

Here is a working example.

200_success · Accepted Answer · 2015-12-28 09:14:35Z

You are using some variables (ul1 and ul2) like "magic" global variables. That is a sloppy IE behaviour that other browsers have copied, and is not recommended. (One reason is that it treats name and id attributes alike.) The fix for that problem is simple: use document.getElementById(…) instead.

Your code is odd, in that the computation occurs at load time; clicking the button merely outputs a copy of the pre-computed result. Is that what you intended?

I recommend splitting the function up into three:

Attaching the button's handler
A function that merges HTML lists
An event handler that calls the merging function with the appropriate lists and outputs the result

In the merge function, I suggest using a hash to detect duplicates. Otherwise, you would have an O(n) search for each item, for a total of O(n²).

window.addEventListener('load', function() {
    document.getElementById('makeNewArray').addEventListener('click', buttonClicked);
});

function buttonClicked() {
    var output = document.createElement('div');
    output.innerHTML = mergeLists([
        document.getElementById('ul1'),
        document.getElementById('ul2'),
    ]);
    body.appendChild(output);
}

function mergeLists(lists) {
    var seen = {};
    var result = [];
    
    for (var i = 0; i < lists.length; i++) {
        var items = lists[i].getElementsByTagName('li');
        for (var j = 0; j < items.length; j++) {
            var str = items[j].innerHTML;
            if (!(str in seen)) {
                result.push(str);
                seen[str] = true;
            }
        }
    }
    return result;
}

<body id="body">
<div>
  <ul id="ul1">
    <li>Любовь</li>
    <li>Надежда</li>
    <li>Вера</li>
  </ul>
</div>
<div>
  <ul id="ul2">
    <li>София</li>
    <li>Надежда</li>
    <li>Вера</li>
  </ul>
</div>
<button id="makeNewArray">Создать новый массив</button>
</body>

Stack Exchange Network

current community

your communities

more stack exchange communities

Extracting distinct HTML list items into a JavaScript array

2 Answers 2

Your Answer

Hot Network Questions

Extracting distinct HTML list items into a JavaScript array

2 Answers 2

Your Answer

Sign up or log in

Post as a guest

Related

Hot Network Questions