4
\$\begingroup\$

Background and Code

I am using the following JavaScript code to inject CSS classes into a webpage after the page has loaded. These classes act as selectors that I can use to pinpoint a table cell to perform other actions (the actions that I wish to perform are out of the scope of this question).

These selectors are used in automated UI testing on a legacy system where tables are used as layout, some tables are often times nested in other tables as layout elements, and unique IDs/names are not available. By injecting CSS classes with JavaScript in this way, I can accurately perform Selenium actions that are resilient (testing is out of the scope of this question the JavaScript that is here and it's performance are the scope of this question).

async function addTableCellClassInjector(row, tableIndex, rowIndex) {
    for (let columnIndex = 0; column = row.cells[columnIndex]; columnIndex++) {
        column.classList.add('agtelc-' + tableIndex + '-' + rowIndex + '-' + columnIndex);
    }
}

async function addTableRowClassInjector(table, tableIndex) {
    for (let rowIndex = 0; row = table.rows[rowIndex]; rowIndex++) {
        row.classList.add('agtelr-' + tableIndex + '-' + rowIndex);
        addTableCellClassInjector(row, tableIndex, rowIndex);
    }
}

async function addTableClassInjector(tables) {
    for (let tableIndex = 0; tableIndex < tables.length; tableIndex++) {
        tables[tableIndex].classList.add('agtelt-' + tableIndex);
        addTableRowClassInjector(tables[tableIndex], tableIndex);
    }
}

addTableRowClassInjector(document.querySelectorAll('table'));

This code performs just fine when there are a couple of tables with a few rows to iterate over. However, the performance slows greatly when there are multiple tables with hundreds of rows.

I believe there might be a better way to handle this whether through reworking the algorithm or using a combination of element selection and the arrays that are return through element selection operations.

Example Usage

Given the following HTML code (inaccuracies and all)

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <title>Convoluted Layout Example</title>
</head>
<body>
    <div>
        <table>
            <tbody>
                <div>
                    <tr>
                        <td><button>Hello</button></td>
                        <td><button>Hello</button></td>
                        <td><button>Hello</button></td>
                        <td><button>Hello</button></td>
                        <td><button>Hello</button></td>
                        <td><button>Hello</button></td>
                    </tr>
                </div>
                <div>
                    <tr>
                        <td><button>Hello</button></td>
                        <td><button>Hello</button></td>
                        <td><button>Hello</button></td>
                        <td><button>Hello</button></td>
                        <td><button>Hello</button></td>
                        <td><button>Hello</button></td>
                    </tr>
                </div>
                <div>
                    <tr>
                        <td><button>Hello</button></td>
                        <td><button>Hello</button></td>
                        <td><button>Hello</button></td>
                        <td><button>Hello</button></td>
                        <td><button>Hello</button></td>
                        <td><button>Hello</button></td>
                    </tr>
                </div>
                <div>
                    <tr>
                        <td><button>Hello</button></td>
                        <td><button>Hello</button></td>
                        <td><button>Hello</button></td>
                        <td><button>Hello</button></td>
                        <td><button>Hello</button></td>
                        <td><button>Hello</button></td>
                    </tr>
                </div>
            </tbody>
        </table>
    </div>
</body>
</html>

I am using Selenium to target a button that is listed on a table. Have made an example with similarities to the actual web page I am testing against. Please note that I cannot change the source of page I am testing against.

In order to click on the 5 button in the fourth row in Selenium, I need to use the following selector (please note that an additional div or span may be added to the row, or cell elements during run time depending on the JavaScript that is running in the background thusly breaking this selection method).

body > div:nth-child(1) > table:nth-child(5) > tbody:nth-child(1) > tr:nth-child(3) > td:nth-child(5) > button:nth-child(1)

By injecting these classes I can target that same element using the following query.

.agtelc-0-3-4 > button

This allows me to accurately target the button in a more stable way.

Question

Are there any code improvements that can be made to this solution that will scale well as tables and rows/columns increase?

I have setup a sample page as a test for the purposes of this question Example page with tables for iteration performance improvements

\$\endgroup\$
9
  • \$\begingroup\$ Welcome to code review! could you please edit your post to include some samples of the CSS that uses the added classes? \$\endgroup\$ Jul 20, 2020 at 3:32
  • 1
    \$\begingroup\$ Why are those functions async? \$\endgroup\$
    – slepic
    Jul 20, 2020 at 5:42
  • \$\begingroup\$ Why are you doing this in the first place? What do you need the classes for? If it's about selecting the cells in CSS, then that can be done with :nth-child instead. \$\endgroup\$
    – RoToRa
    Jul 20, 2020 at 9:02
  • \$\begingroup\$ @RoToRa this is being done for automated UI testing against a legacy system. In the system tables are often used as layout and are often nested. There may be an element on the 4th row of the table which is actually a table nested in another table. To avoid having to use XPath (which is fragile) and fighting against the dynamic elements changing CSS paths, I'm injecting these class selectors into each cell to make it easy and predictable to identify elements. Continued in next comment... \$\endgroup\$ Jul 20, 2020 at 10:12
  • 1
    \$\begingroup\$ Just a thought, what about a happy medium where you only give a class to tables? then your example would be reduced to a pointer to the actual table? And it would be much much faster. \$\endgroup\$
    – konijn
    Jul 20, 2020 at 14:21

1 Answer 1

1
\$\begingroup\$

From a short review;

  • There is no point in creating those functions as async, if anything it gives the reader the mistaken impression that anything async may happen in those functions

  • (let columnIndex = 0; column = row.cells[columnIndex]; columnIndex++) creates a global variable column which is bad practice

  • addTableCellClassInjector is so specific that it currently does not look re-usable at all. As such, I would fold that loop back in to addTableRowClassInjector

  • I dislike the Injector at the end of those function names, addTableCellClass is fine

  • I have the suspicion you do all this to find the column and row when you click a cell, this can all be avoid thusly;

    //With td containing the clicked cell
    //Borrowed from https://stackoverflow.com/questions/16130062/
    const col = td.cellIndex;
    const row = td.parentNode.rowIndex;
    
\$\endgroup\$
4
  • \$\begingroup\$ Seeing how performance is the issue I'm trying to solve, how would I parallelize the execution of steps that are not dependent on previous. Once I have all the tables on the page, one of these functions should be able to work on each of the tables in question independently once it has been assigned a table index. Additionally, each row can index its cells without the previous row needing to have finished its indexing cycle. I may be applying a C# mentality to JavaScript, but I would get a several fold performance boost in C# by doing it this way. Is this achievable in JavaScript? \$\endgroup\$ Jul 20, 2020 at 10:33
  • 1
    \$\begingroup\$ I think we are missing each other’s point, every cell already knows what column it is in and what row, so you don’t have to do this at all. As for the asynchronous approach, you should check out webworkers. \$\endgroup\$
    – konijn
    Jul 20, 2020 at 10:40
  • \$\begingroup\$ Its probably a lack of clarity on my part. I'm updating the question now with more relevant details. I believe I understand going at it from the cell first instead of from the table. I can look at reversing this algorithm, but I think because I left out the usage case its a little bit confusing to everyone. I'm using Selenium or another browser automation tool to click a button in a table that embedded in a table, that's embedded in another table for UI automation testing. However, the DOM changes and is unpredictable, so adding these classes ensures that my automation works predictably. \$\endgroup\$ Jul 20, 2020 at 10:44
  • 1
    \$\begingroup\$ Just updated the question with a use case for context. \$\endgroup\$ Jul 20, 2020 at 11:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.