patternjavascriptMinor

Shorter way of writing my displayError() function?

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

writingfunctionwaydisplayerrorshorter

Problem

Is there a way I can write this function with less code?

jQuery

function displayError(display, string, xhr) {
    var args = arguments;
    var $error = $("div#error");
    var visible = $error.is(":visible");

    if(args.length == 1 && typeof display === "boolean" && display == false) {
        if(visible) {
            $error.slideUp();
        }
    } else if(args.length == 2 && typeof display === "boolean" && display == true && typeof string == "string") {      
        if(visible) {
            $error.fadeOut(function () { 
                $(this).html('(!) '+string).fadeIn();
            });
        } else {
            $error.html('(!) '+string).slideDown();
        }
    } else if(args.length == 3 && typeof display === "boolean" && display == true && typeof string == "string" && typeof xhr === "object") {
        if(visible) {
            $error.fadeOut(function () { 
                $(this).html('('+xhr.status+") "+string).fadeIn();
            });
        } else {
            $error.html('('+xhr.status+") "+string).slideDown();
        }
    }
}

Based on Marco's answer I came up with this:

function displayError(display, string, xhr) {
    var $error = $("div#error");
    var errVisible = $error.is(":visible");
    var errStatus = typeof xhr === "object" ? xhr.status : "!";
    var errMsg = '(' + errStatus + ") " + string;

    if(display) {
        if(errVisible) {
            $error.fadeOut(function() {
                $(this).html(errMsg).fadeIn();
            });
        } else {
            $error.html(errMsg).slideDown();
        }
    } else {
        $error.slideUp();
    }
}

Solution

I don't use Javascript, but i could write it in this way:

function displayError(display, string, xhr) 
{
    var args = arguments;
    var $error = $("div#error");
    var visible = $error.is(":visible");

    if (!display)
    {
        $error.slideUp();
    }
    else
    {

        if (visible) 
        {
            switch (args.length) 
            {
                case 1:
                case 2:
                    $error.fadeOut(function () 
                    {
                        $(this).html('(!) ' + string).fadeIn();
                    });
                    break;
                case 3:
                    $error.fadeOut(function () 
                    {
                        $(this).html('(' + xhr.status + ") " + string).fadeIn();
                    });
                    break;
            }
        } 
        else 
        {
            $error.html('(' + xhr.status + ") " + string).slideDown();
        }
    }
}

But why you use args.length == 1/2/3 if you don't do anything with it?
You could just write:

function displayError(display, string, xhr) {
    var $error = $("div#error");
    var visible = $error.is(":visible");
    var status = typeof xhr === "object" ? xhr.status : "!";

    if (display) {
        if (visible) {
            $error.fadeOut(function () {
                $(this).html('(' + status + ') ' + string).fadeIn();
            });

        } else {
            $error.html('(' + status + ") " + string).slideDown();
        }
    } else {
        $error.slideUp();
    }
}

I used var status = typeof xhr === "object" ? xhr.status : "!"; so you will get the status if xhr is an object, or ! if not.

I removed all typeof parts since i found it useless, you will always get a string so why you worry about it

Code Snippets

function displayError(display, string, xhr) 
{
    var args = arguments;
    var $error = $("div#error");
    var visible = $error.is(":visible");

    if (!display)
    {
        $error.slideUp();
    }
    else
    {

        if (visible) 
        {
            switch (args.length) 
            {
                case 1:
                case 2:
                    $error.fadeOut(function () 
                    {
                        $(this).html('<b style="color: #ce1919;">(!)</b> ' + string).fadeIn();
                    });
                    break;
                case 3:
                    $error.fadeOut(function () 
                    {
                        $(this).html('<b style="color: #ce1919;">(' + xhr.status + ")</b> " + string).fadeIn();
                    });
                    break;
            }
        } 
        else 
        {
            $error.html('<b style="color: #ce1919;">(' + xhr.status + ")</b> " + string).slideDown();
        }
    }
}

function displayError(display, string, xhr) {
    var $error = $("div#error");
    var visible = $error.is(":visible");
    var status = typeof xhr === "object" ? xhr.status : "!";

    if (display) {
        if (visible) {
            $error.fadeOut(function () {
                $(this).html('<b style="color: #ce1919;">(' + status + ')</b> ' + string).fadeIn();
            });

        } else {
            $error.html('<b style="color: #ce1919;">(' + status + ")</b> " + string).slideDown();
        }
    } else {
        $error.slideUp();
    }
}

Context

StackExchange Code Review Q#44057, answer score: 3

Revisions (0)

No revisions yet.