HiveBrain v1.2.0
Get Started
← Back to all entries
snippetjavascriptMinor

jQuery code to create web alert

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
createjquerywebcodealert

Problem

I wrote the following code using jQuery to create a WebAlert instead of using the 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. WebAlert can be simplified since it always
returns false.

The name callback implies the parameter being a function, which it's
not, 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 the
website, 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 immediately
doesn't strike me as a good way. Can't it just stay there and be filled
with new content (.html(msg)) instead of
Remove_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.