3
\$\begingroup\$

I would like to know if this code follows recommended practices and standards for AJAX/PHP calls. Can this AJAX + PHP code be improved in terms of security, performance, etc.?

ajax.js:

function getTableData() {
    const xmlhttp = new XMLHttpRequest();
    xmlhttp.onreadystatechange = function() {
        if (this.readyState === 4 && this.status === 200) {
            document.querySelector('#test-table tbody').innerHTML = this.responseText;
        }
    }
    xmlhttp.open('GET', 'ajax.php', true);
    xmlhttp.send();
}

$(document).ready(function () {
    getTableData();
});

ajax.php:

<?php
$data = prepareDataForTable();
$data_length = count($data);
$columns = 4;

for ($row = 0; $row < $data_length; $row += $columns) {
    echo '<tr>';
        for ($col = 0; $col < $columns && ($col + $row) < $data_length; $col++) {
            echo '<td>' . $data[$col + $row] . '</td>';
        }
    echo '</tr>';
}
?>

ajax.html:

<table id="test-table">
    <tbody>
    </tbody>
</table>

I specifically want to know if this is a good way to send information back to the client - by running loops and echoing output.

\$\endgroup\$
5
  • \$\begingroup\$ All I can say is that the little code you show us could be a lot shorter. You could use JQuery for the ajax call (it does all the complicated & compatibility stuff for you) and if prepareDataForTable() would return a 2D data array you could use 2 simple foreach loops to echo the table. That would even be more flexible than what you have now. \$\endgroup\$ Commented Mar 31, 2019 at 7:50
  • \$\begingroup\$ @KIKOSoftware actually meant something else...Is it fine to echo data like that? Or should I transport it in JSON? - or something along those lines... \$\endgroup\$
    – tera_789
    Commented Mar 31, 2019 at 8:00
  • \$\begingroup\$ @KIKOSoftware Also, I prefer vanilla JS over jQuery. prepareDataForTable() actually returns 1D array but I need to print <tr> tags for every 4th element in array so I thought for loop was a better choice here. If you can demonstrate how to do the same with foreach, then I'll appreciate it. \$\endgroup\$
    – tera_789
    Commented Mar 31, 2019 at 8:13
  • \$\begingroup\$ In this basic code I think you can echo either HTML or JSON, it doesn't really matter. You're not using a MVC framework, or anything like it. Using vanilla JS is brave but also a bit stupid (sorry!). Other people have done the hard work for you, why not use it? Moreso, if JS evolves, which it will, all you have to do is update JQuery, instead of changing all your code. Finally, I know prepareDataForTable() return a 1D array of 2D data. Returning a 2D array will make it easier to handle the data. \$\endgroup\$ Commented Mar 31, 2019 at 8:45
  • \$\begingroup\$ @KIKOSoftware I am actually using vanilla JS here because the project is not too big. I usually go to other libraries/frameworks if the project is big enough. I know it is a big debate whether to use vanilla JS or its libraries:) \$\endgroup\$
    – tera_789
    Commented Mar 31, 2019 at 19:25

1 Answer 1

2
\$\begingroup\$

Trying to follow your mathematical loop conditions makes my eyes go crossed in a hurry.

Perhaps you'd entertain a less-math-heavy alternative:

foreach (array_chunk($array, 4) as $row) {
    echo '<tr><td>' , implode('</td><td>', $row) , '</td>' , str_repeat('<td></td>', 4-count($row)) , '</tr>';
}

Granted this will be making a no-output call of str_repeat() on all rows except potentially the final row, but I think this technique will be more developer friendly. I'm assuming that I understand your $data (Demo). I could have written a conditional for str_repeat() but felt that would only bloat the code.

I tend to lean toward passing json back from an ajax call, but for this situation sending back html is the simpler approach.

\$\endgroup\$
1
  • \$\begingroup\$ Thanks. Can you please expand your answer and show me how to pass JSON back in my case? \$\endgroup\$
    – tera_789
    Commented Mar 31, 2019 at 19:12

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.