patternjavascriptMinor
Getting JSON and putting it in a table using jQuery and PHP
Viewed 0 times
phpandgettingjqueryputtingusingjsontable
Problem
How can I improve this code?
jQuery:
PHP:
jQuery:
$.ajax({
url: "get_attributes.php",
type: "post",
datatype: "json",
data: {
wtype: red_type
},
success: function (data) {
var toAppend = '';
if (typeof data === "object") {
toAppend += "";
toAppend += "Type:" + data['type'] + "";
toAppend += "Health:" + data['health'] + "";
toAppend += "Attack:" + data['attack'] + "";
toAppend += "Defense:" + data['defense'] + "";
toAppend += "Speed:" + data['speed'] + "";
toAppend += "Evade:" + data['evade'] + "";
toAppend += "Special:" + data['special'] + "";
toAppend += "";
$("tbody").remove();
$("#red_form table").append(toAppend);
}
}
});PHP:
Solution
There are a few improvements that come to my mind for the given code:
-
you may want to return an status code for your JSON output instead of checking whether the result is an object. It would allow you to handle error messages and such in a more convenient manner.
-
as a minor improvement, you can use replaceWith() to replace the table body instead of removing it and then appending a new one to the table.
-
you don't have an error handler to your script... if you won't ever need it, that's cool, but it's always best to know when a script fails :)
-
to improve concatenation performance, you might want to use Array's join() method instead of doing
-
you're now removing all table bodies on the whole page, which could be kind of risky if you have more than one table there. Furthermore, the selector is extensively greedy, as jQuery has to find all table bodies on the whole page. It's better to use ID selectors when possible, as they are the fastest one to execute.
-
PHP-wise, you don't handle invalid $wtype, so if no condition is met, you won't know what went wrong, since nothing relevant will be returned
Enough of the theory, this is what I have in mind:
And the PHP:
-
you may want to return an status code for your JSON output instead of checking whether the result is an object. It would allow you to handle error messages and such in a more convenient manner.
-
as a minor improvement, you can use replaceWith() to replace the table body instead of removing it and then appending a new one to the table.
-
you don't have an error handler to your script... if you won't ever need it, that's cool, but it's always best to know when a script fails :)
-
to improve concatenation performance, you might want to use Array's join() method instead of doing
+= for each table row.-
you're now removing all table bodies on the whole page, which could be kind of risky if you have more than one table there. Furthermore, the selector is extensively greedy, as jQuery has to find all table bodies on the whole page. It's better to use ID selectors when possible, as they are the fastest one to execute.
-
PHP-wise, you don't handle invalid $wtype, so if no condition is met, you won't know what went wrong, since nothing relevant will be returned
Enough of the theory, this is what I have in mind:
$.ajax({
url: "get_attributes.php",
type: "post",
datatype: "json",
data: {wtype: red_type},
success: function(data) {
if (data["status"] == 200) {
var toAppend = [];
toAppend.push(
"",
"Type:"+data['type']+"",
"Health:"+data['health']+"",
"Attack:"+data['attack']+"",
"Defense:"+data['defense']+"",
"Speed:"+data['speed']+"",
"Evade:"+data['evade']+"",
"Special:"+data['special']+"",
""
);
$("#red_form table tbody").replaceWith(toAppend.join(""));
} else {
alert('An error has occured. Please try again.');
console.log('Status: ' + data["status"] + ', message: ' + data["message"]);
}
},
error: function(jqXHR, textStatus, errorThrown) {
alert('An error has occured. Please try again.');
console.log('ErrStatus: ' + textStatus + ', error: ' + errorThrown);
}
});And the PHP:
200, 'message' => 'ok', 'type' => $wtype);
switch ($wtype) {
case 'Ninja':
$attributes['health'] = '40-60';
$attributes['attack'] = '60-70';
$attributes['defense'] = '20-30';
$attributes['speed'] = '90-100';
$attributes['evade'] = '0.3-0.5';
$attributes['special'] = 'There is a 5% chance of 2x attack';
break;
case 'Samurai':
$attributes['health'] = '60-100';
$attributes['attack'] = '75-80';
$attributes['defense'] = '35-40';
$attributes['speed'] = '60-80';
$attributes['evade'] = '0.3-0.4';
$attributes['special'] = '10% chance of restoring +10 health when evade is successful';
break;
case 'Brawler':
$attributes['health'] = '90-100';
$attributes['attack'] = '65-75';
$attributes['defense'] = '40-50';
$attributes['speed'] = '40-65';
$attributes['evade'] = '0.3-0.35';
$attributes['special'] = 'increased +10 defense when health is below 20%';
break;
default: $attributes['status'] = 400;
$attributes['message'] = 'Invalid class info has been requested.';
}
echo json_encode($attributes);
?>Code Snippets
$.ajax({
url: "get_attributes.php",
type: "post",
datatype: "json",
data: {wtype: red_type},
success: function(data) {
if (data["status"] == 200) {
var toAppend = [];
toAppend.push(
"<tbody>",
"<tr></td><td class=datalabel>Type:</td><td>"+data['type']+"</td></tr>",
"<tr><td class=datalabel>Health:</td><td>"+data['health']+"</td></tr>",
"<tr><td class=datalabel>Attack:</td><td>"+data['attack']+"</td></tr>",
"<tr><td class=datalabel>Defense:</td><td>"+data['defense']+"</td></tr>",
"<tr><td class=datalabel>Speed:</td><td>"+data['speed']+"</td></tr>",
"<tr><td class=datalabel>Evade:</td><td>"+data['evade']+"</td></tr>",
"<tr><td class=datalabel>Special:</td><td>"+data['special']+"</td></tr>",
"</tbody>"
);
$("#red_form table tbody").replaceWith(toAppend.join(""));
} else {
alert('An error has occured. Please try again.');
console.log('Status: ' + data["status"] + ', message: ' + data["message"]);
}
},
error: function(jqXHR, textStatus, errorThrown) {
alert('An error has occured. Please try again.');
console.log('ErrStatus: ' + textStatus + ', error: ' + errorThrown);
}
});<?php
header("Content-Type: application/json");
$wtype = (isset($_POST['wtype']) ? $_POST['wtype'] : 'unknown');
$attributes = array('status' => 200, 'message' => 'ok', 'type' => $wtype);
switch ($wtype) {
case 'Ninja':
$attributes['health'] = '40-60';
$attributes['attack'] = '60-70';
$attributes['defense'] = '20-30';
$attributes['speed'] = '90-100';
$attributes['evade'] = '0.3-0.5';
$attributes['special'] = 'There is a 5% chance of 2x attack';
break;
case 'Samurai':
$attributes['health'] = '60-100';
$attributes['attack'] = '75-80';
$attributes['defense'] = '35-40';
$attributes['speed'] = '60-80';
$attributes['evade'] = '0.3-0.4';
$attributes['special'] = '10% chance of restoring +10 health when evade is successful';
break;
case 'Brawler':
$attributes['health'] = '90-100';
$attributes['attack'] = '65-75';
$attributes['defense'] = '40-50';
$attributes['speed'] = '40-65';
$attributes['evade'] = '0.3-0.35';
$attributes['special'] = 'increased +10 defense when health is below 20%';
break;
default: $attributes['status'] = 400;
$attributes['message'] = 'Invalid class info has been requested.';
}
echo json_encode($attributes);
?>Context
StackExchange Code Review Q#15756, answer score: 5
Revisions (0)
No revisions yet.