Randomly rearrange table rows without unnecessary reflows

Question

I recently completed a coding challenge for a company that did not get me to the next stage. They were kind enough to provide the following feedback:

The coding assignment does not match what we are looking for, a mid-level Front End Engineer. The code does not fully comply with what is asked in the assignment and the code readability is sometimes hard.

I have no problems with these criticisms as I am still in my first year as a professional developer. However, I am really curious as to how a mid-level engineer would approach the problem. Any advice on how to improve readability would also be greatly appreciated (it's a mess in some spots I know...). I am not sure which sections they are referring to that do not comply with what is asked in the assignment.

I will list both the code and the requirements below.

Requirements

When index.html is opened in the browser, the content of constant USERS in json/data.js will appear as a table on the browser. Table column order should be the same as the order already written in index.html.
- Name is the last name and the first name, respectively, separated by a space.
When button#start is clicked, table rows will randomly rearrange every second.
The process in 2 can be stopped when button#stop is clicked.
When user name very long (left to your own appreciation), an ellipsis is shown.
When button#sort is clicked, rows will be sorted by balance in descending order. If two or more balances are the same, rows will be sorted in ascending id order.
When the process in 2-3 is running, disable button#sort.

Additional Notes

Please write your code so as to avoid triggering unnecessary reflows and repaints.

... and here is the code:

JS

class App {
    constructor(props) {
        this.startRandomBtn = document.getElementById('start');
        this.stopRandomBtn = document.getElementById('stop');
        this.sortTableBtn = document.getElementById('sort');
        this.table  = document.querySelector('.table');
        this.data = props.users;
        this.active;
        this.init();
    }

    sortTable() {
        let switching, shouldSwitch, rows, i
        switching = true

        while (switching) {
            switching = false;
            rows = this.table.rows;

            for (i = 1; i < (rows.length - 1); i++) {
              shouldSwitch = false;
              let balanceX = rows[i].querySelector('.balance').textContent
              let balanceY = rows[i + 1].querySelector('.balance').textContent;
              let idX = rows[i].getAttribute('data-id')
              let idY = rows[i + 1].getAttribute('data-id')

              if (parseInt(balanceX) === parseInt(balanceY)) {
                if (parseInt(idX) < parseInt(idY)) {
                    shouldSwitch = true;
                    break;
                }
              }

              if (parseInt(balanceX) < parseInt(balanceY)) {
                shouldSwitch = true;
                break;
              }
            }
            if (shouldSwitch) {
                rows[i].parentNode.insertBefore(rows[i + 1], rows[i]);
                switching = true;
              }
        }
    }

    setTable() {
        this.data.forEach(item => {

            let trNode = document.createElement('tr');

            for (const key in item) {
                if (key === "id") {
                    trNode.setAttribute('data-id', item[key])
                }
                else if (key === "name") {
                    let tdNode = document.createElement('td');
                    let firstName = item[key].first
                    let lastName = item[key].last

                    if (lastName.length > 11) {
                        lastName = lastName.slice(0,11).concat("...");
                    }

                    tdNode.textContent = `${lastName}, ${firstName}`
                    trNode.appendChild(tdNode)
                } 
                else if (key === "pictureUrl") {
                    let img = document.createElement('img')
                    img.src = `${item[key]}`

                    trNode.appendChild(img)
                }
                
                else {
                    let tdNode = document.createElement('td');
                    tdNode.textContent = `${item[key]}`
                    if (key === "accountBalance") {
                        tdNode.classList.add("balance")
                    }

                    trNode.appendChild(tdNode)
                }
              }
              this.table.appendChild(trNode)     
        })
    }

    stopRandomize () {
        clearInterval(this.active);
        this.sortTableBtn.classList.remove("disabled");
        this.sortTableBtn.style.pointerEvents = "auto";
    }

    randomizeArray() {

        // Disable sort button while list is being randomized
        this.sortTableBtn.classList.add("disabled");
        this.sortTableBtn.style.pointerEvents = "none";

        this.active = setInterval(() => {
            let rowsCollection = document.querySelectorAll('tr');
            let rows = Array.from(rowsCollection).slice(1);

            for (let i = rows.length - 1; i > 0; i--) {
                let j = Math.floor(Math.random() * (i + 1));
                let temp = rows[i];
                rows[i] = rows[j];
                rows[j] = temp;
            }

            for (const row of rows) {
                this.table.appendChild(row);
            }
        }, 1000)
    }

    init() {
        // Set up event listeners
        this.startRandomBtn.addEventListener('click', () => this.randomizeArray());
        this.stopRandomBtn.addEventListener('click', () => this.stopRandomize());
        this.sortTableBtn.addEventListener('click', () => this.sortTable());

        // Initialize Table
        this.setTable();
    }
}

new App({
    users: USERS 
});

HTML

<form role="form">
        <div class="form-group mb-5 d-flex justify-content-center">
          <button id="start" type="button" class="btn btn-success mx-3">start random</button>
          <button id="stop" type="button" class="btn btn-success mx-3">stop random</button>
          <button id="sort" type="button" class="btn btn-success mx-3">sort</button>
        </div>
      </form>

      <div class="contents">
        <table class="table ">
          <thead>
            <tr>
              <th scope="col">Name</th>
              <th scope="col">Email</th>
              <th scope="col">Phone</th>
              <th scope="col">Picture</th>
              <th scope="col">Balance</th>
            </tr>
          </thead>
          <tbody>
          </tbody>
        </table>
      </div>

I will include the json data as well, in case its of interest

const USERS = [
  {
    "id": "5",
    "name": {
      "title": "Mr",
      "first": "Pitágoras",
      "last": "Fogaça"
    },
    "email": "[email protected]",
    "phone": "(77) 7242-0589",
    "pictureUrl": "image/1.jpg",
    "accountBalance": 100
  },
  {
    "id": "7",
    "name": {
      "title": "Miss",
      "first": "Maddison",
      "last": "Mitchell"
    },
    "email": "[email protected]",
    "phone": "051-630-3570",
    "pictureUrl": "image/2.jpg",
    "accountBalance": 150
  },
  {
    "id": "2",
    "name": {
      "title": "Mr",
      "first": "Landon",
      "last": "Caldwell"
    },
    "email": "[email protected]",
    "phone": "(481)-936-9008",
    "pictureUrl": "image/3.jpg",
    "accountBalance": 120
  },
  {
    "id": "9",
    "name": {
      "title": "Mr",
      "first": "Lee",
      "last": "Chapman"
    },
    "email": "[email protected]",
    "phone": "063-85445367",
    "pictureUrl": "image/4.jpg",
    "accountBalance": 110
  },
  {
    "id": "10",
    "name": {
      "title": "Mr",
      "first": "Lewis",
      "last": "Overgoor"
    },
    "email": "[email protected]",
    "phone": "(835)-339-5805",
    "pictureUrl": "image/5.jpg",
    "accountBalance": 80
  },
  {
    "id": "1",
    "name": {
      "title": "Ms",
      "first": "Venla",
      "last": "Korpi"
    },
    "email": "[email protected]",
    "phone": "06-156-153",
    "pictureUrl": "image/6.jpg",
    "accountBalance": 190
  },
  {
    "id": "3",
    "name": {
      "title": "Mr",
      "first": "Hubert Blaine",
      "last": "Wolfeschlegelsteinhausenbergerdorff"
    },
    "email": "[email protected]",
    "phone": "003-16911265",
    "pictureUrl": "image/7.jpg",
    "accountBalance": 100
  },
  {
    "id": "8",
    "name": {
      "title": "Miss",
      "first": "Maria",
      "last": "Christensen"
    },
    "email": "[email protected]",
    "phone": "87931526",
    "pictureUrl": "image/8.jpg",
    "accountBalance": 190
  },
  {
    "id": "4",
    "name": {
      "title": "Mr",
      "first": "Philip",
      "last": "Brown"
    },
    "email": "[email protected]",
    "phone": "<img onerror='window.document.body.innerHTML = \"<h1>XSS</h1>\";' src=''> ",
    "pictureUrl": "image/9.jpg",
    "accountBalance": 120
  },
  {
    "id": "6",
    "name": {
      "title": "Mr",
      "first": "Viljami",
      "last": "Wallo"
    },
    "email": "[email protected]",
    "phone": "06-616-693",
    "pictureUrl": "image/10.jpg",
    "accountBalance": 95
  }
];

Thank you for taking the time to look!

"The code does not fully comply with what is asked in the assignment" Please clarify: does or doesn't the code produce the required output? — Mast
– Mast ♦, Commented Apr 13, 2021 at 6:05
Actually, the code doesn't meet the requirements. Q4. Name is defined as a combination of first and last. You only abbreviated the last name. Q5, The code only sorts by amount and not by ascending ids. Additional notes were not applied. Everytime you append a row, you move it and cause a reflow/paint. You should have worked in a document fragment to avoid this. — TinMan
– TinMan, Commented Apr 13, 2021 at 6:13
The current question title, which states your concerns about the code, applies to too many questions on this site to be useful. The site standard is for the title to simply state the task accomplished by the code. Please see How do I ask a good question?. — BCdotWEB
– BCdotWEB, Commented Apr 13, 2021 at 7:07
Thanks for the links @BCdotWEB, I will try to frame the question title better. As for the code working correctly, to the best of my knowledge it was working at the time of posting. It was only after it was pointed out by other commenters that I realized that I did not fulfill the requirements as expected. — j-scu486
– j-scu486, Commented Apr 13, 2021 at 7:10

radarbob · Accepted Answer · 2021-04-30 17:42:00Z

The code does not fully comply with what is asked in the assignment and the code readability is sometimes hard.

These critiques will be corrected throughout the code when we implement the fundamental idea to ...

Manipulate the data, not the display

Take this as a fundamental principle. Many OO design and coding buzz-word benefits naturally flow from this.

Once the USERS data is formatted, sorted, etc. then render the data in its entirety, completely replacing the current table.

Separation of Concerns & Single Responsibility Principle

The data fetch, preparation, and display become completely independent. The data - Users - is sorted, etc. without any awareness of display. The html is only displaying without manipulating the data in any way.

USERS as a 'collection-of-User' object

~~Copy the USERS json because it is constant and this will separate (the concerns of) data fetch from particular desired formatting.~~ On second thought, the JSON does not necessarily need to be copied. const USERS = [ ... ] only means the object referenced cannot be replaced. However, the data, properties, etc. within that object can be changed. Perhaps knowing or taking advantage of this feature is part of the test. This would certainly simplify getting changed USERS data back out.

Users will contain this array along with methods like randomizeOrder. The Users object will iterate itself to do everything. Client code will only call the methods.

Array methods like sort and forEach is going to simplify the hell out the current code. Code readability will POP from the page.

Do all value formatting and editing on the array, not the table. Never the table.

Naming

Name things for what they are and never use data type as part of the name. User is a single person. Users is a collection of persons.

When Users has its own methods to "orderByBalance", for example, then we don't need to name our variables as reminders to use Javascript's Array methods.

Stack Exchange Network

Randomly rearrange table rows without unnecessary reflows

Requirements

Additional Notes

JS

HTML

1 Answer 1

You must log in to answer this question.

Hot Network Questions

Randomly rearrange table rows without unnecessary reflows

Requirements

Additional Notes

JS

HTML

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions