patternjavascriptMinor
More maintainable API for wysihtml5 widget
Viewed 0 times
morewysihtml5maintainablewidgetforapi
Problem
For an HTML calendar with jQuery, I put in event listeners to enable jQuery drag-and-drop that reads and writes to persistence using HTTP get and post with JSON structures.
Now I'd like to make the code more readable and easier to follow and understand. Can you let me know suggestions on how to improve? My plan is to make smaller moduler with clear responsibilities. The purpose of the script is to enable persistent data in jQuery wysihtml5 window. I use PHP from my previous question where I wrote a PHP handler to create a JSON response.
My development plan is to rewrite the jQuery code that uses the JSON response so that the code below will be structured into smaller and clearer units and maybe in a separate file.
```
$(document).ready(function() {
// Allhelgonafton Nu firar man!
$('#holidays').bind("DOMSubtreeModified", function(event) {
alert('test');
if ($('#modal-droppable-0 > img').attr('id') > 0) {
}
});
/ Text area functionality TODO: per user, per month, many images, åäö, popup rendering, prepopulate här kan du skriva /
$('#modal-droppable-0').bind("DOMSubtreeModified", function(event) {
if ($('#modal-droppable-0 > img').attr('id') > 0) {
var date_month = $('.dateDayModal > h2').text();
$.post("php/updateImage.php", {
image_id: $('#modal-droppable-0 > img').attr('id'),
day: date_month,
u
Now I'd like to make the code more readable and easier to follow and understand. Can you let me know suggestions on how to improve? My plan is to make smaller moduler with clear responsibilities. The purpose of the script is to enable persistent data in jQuery wysihtml5 window. I use PHP from my previous question where I wrote a PHP handler to create a JSON response.
function getDateDetails($day, $month, $year, $username, $db)
{
$sql = query($sql))
{
die('There was an error running the query [' . $db->error . ']');
}
$ret = array();
while ($row = $result->fetch_assoc())
{
$names = array_filter(explode(";", $row['url']));
$row_array = array(
'id' => $row['image_id'],
'name_day' => $row['url']
);
array_push($ret,$row_array);
}
$ret = json_encode($ret);
return ($ret);
}My development plan is to rewrite the jQuery code that uses the JSON response so that the code below will be structured into smaller and clearer units and maybe in a separate file.
```
$(document).ready(function() {
// Allhelgonafton Nu firar man!
$('#holidays').bind("DOMSubtreeModified", function(event) {
alert('test');
if ($('#modal-droppable-0 > img').attr('id') > 0) {
}
});
/ Text area functionality TODO: per user, per month, many images, åäö, popup rendering, prepopulate här kan du skriva /
$('#modal-droppable-0').bind("DOMSubtreeModified", function(event) {
if ($('#modal-droppable-0 > img').attr('id') > 0) {
var date_month = $('.dateDayModal > h2').text();
$.post("php/updateImage.php", {
image_id: $('#modal-droppable-0 > img').attr('id'),
day: date_month,
u
Solution
For the PHP code:
-
Read up on using prepared statements so you don't get any inadvertant SQL injection
-
The
Other than that the code isn't very pretty but it gets the job done. It could be improved by splitting it up into functions (or class methods) with separate concerns so you don't need to pass the DB object to the same function you feed the
day to...
For the jQuery code:
The following example apply this to the #calendarAnchor click event handler.
It may look/feel more verbose but having more smaller blocks helps re-usability and readability immensely
-
Read up on using prepared statements so you don't get any inadvertant SQL injection
-
The
$month and $year parameters don't seem to be used anywhere.- The
$isFirstand$namesvariable does't seem to be used anywhere.
- Why use
die? Why not just put the error in a JSON string and return that?
Other than that the code isn't very pretty but it gets the job done. It could be improved by splitting it up into functions (or class methods) with separate concerns so you don't need to pass the DB object to the same function you feed the
day to...
For the jQuery code:
- For anything that is repeated more than once, you should consider putting it in a variable.
username: 'bd107a66ba'is a prime candidate. So are various recurring jQuery selectors.
- Blocks of code that look almost the same except for a number (0/1/2/) could just be one function with that number as a parameter. That would reduce your line's of code quite a bit...
- Splitting long lines of code into several lines helps readability
- in regards to creating smaller units, instead of having nested functions, try declaring your functions first and reference them by name from your event handlers
The following example apply this to the #calendarAnchor click event handler.
It may look/feel more verbose but having more smaller blocks helps re-usability and readability immensely
var count;
function prependImage(i, obj) {
if (i == 'id') {
id = obj.id;
}
if (count != 0) {
count = 1;
}
$('#library-' + obj.id).empty();
$('#modal-droppable-' + count).prepend(
''
);
count++;
}
function prependImagesFromJsonData(data) {
count = 0;
$.each(data, prependImage);
}
function setValuesFromJsonData(data) {
$.each(data, function(i, obj) {
if (i == 'dateDescription') {
editor.setValue('' + obj, true);
}
});
}
function buildUrl(url, event) {
return 'php/' + url + '.php' +
'?action=GET_DATE_DETAILS' +
'&day=' + $(event.target).text() +
'&month=11&year=2014&username=bd107a66ba'
;
}
function calendarAnchorClickHandler {
$.getJSON(buildUrl('jsonHandler', event), setValuesFromJsonData);
$.getJSON(buildUrl('getImagesJson', event), prependImagesFromJsonData);
}
$("#calendarAnchor").click(calendarAnchorClickHandler);Code Snippets
var count;
function prependImage(i, obj) {
if (i == 'id') {
id = obj.id;
}
if (count != 0) {
count = 1;
}
$('#library-' + obj.id).empty();
$('#modal-droppable-' + count).prepend(
'<img id="' + obj.id + '"'+
' src="' + obj.name_day + '"' +
' data-parent-icon-id="library-' + obj.id + '"' +
' class="icon ui-draggable ui-draggable-handle"' +
' style="position: relative; z-index: 999; left: 0px; top: 0px;"' +
' data-slot="modal-droppable-' + count + '"' +
'/>'
);
count++;
}
function prependImagesFromJsonData(data) {
count = 0;
$.each(data, prependImage);
}
function setValuesFromJsonData(data) {
$.each(data, function(i, obj) {
if (i == 'dateDescription') {
editor.setValue('' + obj, true);
}
});
}
function buildUrl(url, event) {
return 'php/' + url + '.php' +
'?action=GET_DATE_DETAILS' +
'&day=' + $(event.target).text() +
'&month=11&year=2014&username=bd107a66ba'
;
}
function calendarAnchorClickHandler {
$.getJSON(buildUrl('jsonHandler', event), setValuesFromJsonData);
$.getJSON(buildUrl('getImagesJson', event), prependImagesFromJsonData);
}
$("#calendarAnchor").click(calendarAnchorClickHandler);Context
StackExchange Code Review Q#67496, answer score: 3
Revisions (0)
No revisions yet.