2
\$\begingroup\$

I would appreciate feedback especially if you would be embarrassed by this code, or if I was on your team and you would expect better and/or you would mention something.

This code was derived from comments made on Function To Add DOM Nodes (it also lead to new ideas that are unique to this post).

If there are things that could improve performance definitely suggest them (as this code will be called frequently and there will be a lot of data in the browser).

If there is code that should be reused just point it out for me!

This is a javascript function that will accept an array of objects (from a database or websocket) and then spit out HTML <ul> lists.

I also tried to make the code more robust by allowing an array of objects or a single object to be sent and the object(s) properties to be sent in any order but still maintain order for the HTML output.

The code works -- but I am "self-taught" and always learning. I want to be producing professional looking/behaving code and the feedback here will inform all my projects.

Any insights into style or approach would be welcomed. Get the red marker out!

Javascript Code

function addChildren(children) {
    // Add a single child to the DOM (if array not passed)
    if (!Array.isArray(children)) {
        addChildToDOM(children);
        return;
    }

    // Add children to the DOM
    for (const child of children) {
        addChildToDOM(child);
    }
}

// Order the child's DOM elements
/* Places the properties in the correct order -- but you can override the default order.
   Noteably if there are missing properties they will appear at the end of the object (to improve code maintainability) */
function orderChildObject(child, propertyOrder= ["group","flags","firstName","lastName","guardian","checkIn","notes","phone"]) {
    let objectOrder = new Object();
    propertyOrder.forEach(function(value, index, array) {
        objectOrder[value] = null;
    });
    objectOrder = Object.assign(objectOrder, child);
    return objectOrder;
}

function addChildToDOM(child) {
    // Ensure that required properties exist or stop
    if (!(child.hasOwnProperty("firstName") || child.hasOwnProperty("lastName"))
        || !child.hasOwnProperty("phone")) {
        // @TODO add warning to user interface
        console.log("Child missing required data and could not be displayed");
        return;
    }

    // Ensure properties are ordered properly in the DOM
    child = orderChildObject(child);

    // Function variables
    const childList = document.getElementById("childList");  // Top-level DOM node for child nodes
    const childId = child.firstName + child.lastName + child.phone;
    let ul;  // Top-level of this child's node on the DOM

    // Delete child from DOM if overwriting
    if(document.getElementById(child.id)){
        ul = document.getElementById(child.id);
        ul.remove();
    }

    // Add ul child to DOM
    ul = document.createElement("ul");  // Top-level of child on DOM
    ul.id = childId;
    childList.appendChild(ul);

    // Assign <ul> to a group
    if (child.group) {
        ul.classList.add(child.group);
    } else {
        ul.classList.add("unassigned");
    }

    // Add the properties to the child as <li>s
    let childKeys = Object.keys(child);
    for (let property in childKeys) {
        const key = childKeys[property];
        const value = child[childKeys[property]];
        if(key === "id") continue;  // Skip adding the id as an <li> (added as #id on <ul>)
        const li = assignChildProperty(key, value);
        ul.appendChild(li);
    }

    function assignChildProperty(key, value) {

        // If child property value is an object
        if (typeof (value) === "object") {

            // Add <li> to child's <ul> node and create a <ul> for this property
            const outerli = document.createElement("li");
            outerli.classList.add(key);
            const ul = document.createElement("ul");

            // Run through each of the nested objects and add <li>
            for (let property in value) {
                const li = document.createElement("li");
                li.innerText = value[property]; // the object's value
                li.classList.add(key); // the outer object's class key

                // if the object is an array, don't add the array index to class
                if (!Array.isArray(value)) {
                    li.classList.add(property); // the object's key
                }

                // add the <li> to the <ul>
                ul.appendChild(li);
            }

            // add the <ul> to the outer <li> to be returned to the outer <ul>
            outerli.appendChild(ul);
            return outerli;
        }

        // If child property is a simple value (ie. string)
        const li = document.createElement("li");
        li.classList.add(key);
        li.innerText = value;
        return li;
    }
}

The input from an AJAX call or a websocket push:

// example use of data from AJAX database call being implemented
addChildren([
    {
        group: 'prek',
        firstName: "Bobby",
        lastName: "Fisher",
        group: "prek",
        checkIn: "9:14am",
        phone: "ewqrqr3452",
    },
    {
        group: "grade1",
        firstName: "Anne",
        lastName: "Gables",
        guardian: {firstName: "Green", lastName: "Gables"},
        flags: ['peanuts','bees'],
        checkIn: "9:14am",
        phone: "ewqrqr3452",
        test: "this is a test",
    }
]);

// example use of data from a websocket push that would overwrite a node
addChildToDOM(    {
    id: "BobbyFisherewqrqr3452",
    group: "grade1",
    firstName: "Bobby",
    lastName: "Fisher",
    guardian: {firstName: "Green", lastName: "Gables"},
    checkIn: "9:14am",
    phone: "ewqrqr3452",
    test2: "this is a second test",
});

HTML Output

<main id="childList"><!-- where the output hooks on to the HTML -->
  <ul id="BobbyFisherewqrqr3452" class="prek">
    <li class="group">prek</li>
    <li class="firstName">Bobby</li>
    <li class="lastName">Fisher</li>
    <li class="guardian">
      <ul>
        <li class="guardian firstName">Green</li>
        <li class="guardian lastName">Gables</li>
      </ul>
    </li>
    <li class="checkIn">9:14am</li>
    <li class="phone">ewqrqr3452</li>
  </ul>
</main>

Note: I am using Babel for transpiling (added)

\$\endgroup\$
0

1 Answer 1

2
\$\begingroup\$

You have a big problem in your code: Object properties by definition don't have an intrinsic order. What you are doing in orderChildObject may not work. (Strictly speaking the most recent specifications do define an order, but it isn't necessarily insertion order, nor is it used in all cases, nor can you know if the JavaScript engine actually implements it.)

(I don't have time to write more right now. Maybe I can back later.)

\$\endgroup\$
4
  • \$\begingroup\$ I look forward to it! I should mention that all the JS code runs through Bable transpiler. Does that make a difference? \$\endgroup\$ Feb 28, 2020 at 11:03
  • \$\begingroup\$ I had been under the impression that using a transpiler might preserver string insertion order -- because of newer JS. However: Note: Since ECMAScript 2015, objects do preserve creation order for string and Symbol keys. In JavaScript engines that comply with the ECMAScript 2015 spec, iterating over an object with only string keys will yield the keys in order of insertion. (developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/…). \$\endgroup\$ Feb 28, 2020 at 11:20
  • \$\begingroup\$ Perhaps I should become more familiar with Maps. \$\endgroup\$ Feb 28, 2020 at 11:21
  • \$\begingroup\$ Just a note: There is 97%+ (93% globably) estimated browser compatibility with the Object.assign() method (which guarantees string keys to be in the order of insertion) and ES6 specifications for the North American market. Most notably is that there is no compatibility with IE. If you're using a different browser and it has been updated in the last 3+ years: it works (except Opera Mini). Each developer will need to decide basd on their project -- this project probably doesn't need > 93% but I will look into maps because: afterall, it's learning! (and can become part of my own best practice) \$\endgroup\$ Feb 28, 2020 at 16:32

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.