snippetjavascriptMinor
jQuery code to create web alert
Viewed 0 times
createjquerywebcodealert
Problem
I wrote the following code using jQuery to create a WebAlert instead of using the
But I think that it's too complex and non-standard, is that true?
alert() function from jQuery:(function($){
$.fn.WebAlert = function(msg) {
// If WebAlert is Availble -> First Remove this-> callback = Yes -> Now Create New WebAlert
if ($('.WebAlert_Area').length) {
$(this).Remove_WebAlert(msg , 'Yes')
return false;
//If WebAlert Not Availble
} else {
$(this).Create_WebAlert(msg)
return false;
}
}
//Function For Create WebAlert
$.fn.Create_WebAlert = function(msg) {
$('body').append('');
$('.WebAlert_Area .Text_Area').html(msg)
};
//Function For Remove WebAlert
$.fn.Remove_WebAlert = function(msg , callback) {
$('.WebAlert_Area').hide(1000 , function() {
$(this).remove();
if (callback == 'Yes') {
$(this).Create_WebAlert(msg)
}
})
};
})(jQuery);
$(document).ready(function() {
$('body').WebAlert('Welcome To My Web')
}};But I think that it's too complex and non-standard, is that true?
Solution
Consistency and correct syntax would be a start: The last function call
is improperly closed, so technically that's broken. Also my JavaScript
mode marks all missing semicolons, so I inserted them too. The naming
convention is unusual in the sense that it's mixing camel case and
underscores, doesn't loook the greatest. The comments mostly state what
the code already says - I'd leave them out unless they add significant
insights.
Now to the actual code.
returns
The name
not, a better name could be
The whole adding/removing nodes just for an alert seems messy. I'd say
that you should always have the
website, then toggle its visibility with CSS unless there's a pressing
reason to do it this way.
In any case, removing the
doesn't strike me as a good way. Can't it just stay there and be filled
with new content (
Of course it is untested, but I think splitting it like below makes more
sense and is less convoluted:
is improperly closed, so technically that's broken. Also my JavaScript
mode marks all missing semicolons, so I inserted them too. The naming
convention is unusual in the sense that it's mixing camel case and
underscores, doesn't loook the greatest. The comments mostly state what
the code already says - I'd leave them out unless they add significant
insights.
Now to the actual code.
WebAlert can be simplified since it alwaysreturns
false.The name
callback implies the parameter being a function, which it'snot, a better name could be
inCallback or so?The whole adding/removing nodes just for an alert seems messy. I'd say
that you should always have the
div for WebAlert_Area in thewebsite, then toggle its visibility with CSS unless there's a pressing
reason to do it this way.
In any case, removing the
div just to recreate it immediatelydoesn't strike me as a good way. Can't it just stay there and be filled
with new content (
.html(msg)) instead ofRemove_WebAlert(); Create_WebAlert?Of course it is untested, but I think splitting it like below makes more
sense and is less convoluted:
(function($){
$.fn.WebAlert = function(msg) {
if (!$('.WebAlert_Area').length) {
$(this).Create_WebAlert();
}
$(this).Set_WebAlert(msg);
return false;
};
$.fn.Create_WebAlert = function(msg) {
$('body').append('');
};
$.fn.Set_WebAlert = function(msg) {
$('.WebAlert_Area .Text_Area').html(msg);
};
$.fn.Remove_WebAlert = function() {
$('.WebAlert_Area').hide(1000, function() {
$(this).remove();
});
};
})(jQuery);
$(document).ready(function() {
$('body').WebAlert('Welcome To My Web');
});Code Snippets
(function($){
$.fn.WebAlert = function(msg) {
if (!$('.WebAlert_Area').length) {
$(this).Create_WebAlert();
}
$(this).Set_WebAlert(msg);
return false;
};
$.fn.Create_WebAlert = function(msg) {
$('body').append('<div class="WebAlert_Area"><div class="Msg_Area"></div></div>');
};
$.fn.Set_WebAlert = function(msg) {
$('.WebAlert_Area .Text_Area').html(msg);
};
$.fn.Remove_WebAlert = function() {
$('.WebAlert_Area').hide(1000, function() {
$(this).remove();
});
};
})(jQuery);
$(document).ready(function() {
$('body').WebAlert('Welcome To My Web');
});Context
StackExchange Code Review Q#105993, answer score: 4
Revisions (0)
No revisions yet.