1
\$\begingroup\$

I have the following functions that for the most part are utilizing much of the same code (except for the actual message). What are the possibilities, if there are any, for combining / reducing these functions so that there isn't so much redundancy?

    // ACCOUNT DISABLED FUNCTION

    function accountDisabled() {
        if (!$('.accountautologoff').length) {

            $('.content').append('<div class="accountautologoff"><span>Your account has been disabled</span><br /><span>Contact the system administrator</span></div>');

            $('body').addClass('disabled');
            $('html').removeClass('loading');
            $('.accountautologoff').hide().fadeTo(600, 1, 'swing');
            $(document).bind('mousewheel', function (e) { e.preventDefault(); });
            $('.content-home, .content-search').fadeTo(600, 0.5, 'swing');
        };
    };

    // ACCOUNT DELETED FUNCTION

    function accountDeleted() {
        if (!$('.accountautologoff').length) {

            $('.content').append('<div class="accountautologoff"><span>Your account has been removed</span><br /><span>Contact the system administrator</span></div>');

            $('body').addClass('disabled');
            $('html').removeClass('loading');
            $('.accountautologoff').hide().fadeTo(600, 1, 'swing');
            $(document).bind('mousewheel', function (e) { e.preventDefault(); });
            $('.content-home, .content-search').fadeTo(600, 0.5, 'swing');
    };
};
\$\endgroup\$
1
\$\begingroup\$

You could start by actually trying to combine them yourself. :/ we appreciate code reviews but honestly this was a copy and paste job.

function accountDeactivate(msg) {
  if (!$('.accountautologoff').length) {
    $('.content').append('<div class="accountautologoff"><span>' + msg +'</span></div>');
    $('body').addClass('disabled');
    $('html').removeClass('loading');
    $('.accountautologoff').hide().fadeTo(600, 1, 'swing');
    $(document).bind('mousewheel', function (e) { e.preventDefault(); });
    $('.content-home, .content-search').fadeTo(600, 0.5, 'swing');
  };
}
\$\endgroup\$
1
\$\begingroup\$

You could extract similar code-blocks to functions:

function accountDisabled() {
    if (!$('.accountautologoff').length) {
        $('.content').append('<div class="accountautologoff"><span>Your account has been disabled</span><br /><span>Contact the system administrator</span></div>');
        doTheStuff();
    };
};

function accountDeleted() {
    if (!$('.accountautologoff').length) {
        $('.content').append('<div class="accountautologoff"><span>Your account has been removed</span><br /><span>Contact the system administrator</span></div>');
        doTheStuff();
};

function doTheStuff() {
        $('body').addClass('disabled');
        $('html').removeClass('loading');
        $('.accountautologoff').hide().fadeTo(600, 1, 'swing');
        $(document).bind('mousewheel', function (e) { e.preventDefault(); });
        $('.content-home, .content-search').fadeTo(600, 0.5, 'swing');
}

Or you could pass the thing that changes into a common function:

function accountDisabled() {
    accountDeletedOrDisabled('<div class="accountautologoff"><span>Your account has been disabled</span><br /><span>Contact the system administrator</span></div>');
}

function accountDeleted(errorMessage) {
    accountDeletedOrDisabled('<div class="accountautologoff"><span>Your account has been removed</span><br /><span>Contact the system administrator</span></div>');
}

function accountDeletedOrDisabled(errorMessage) {
    if (!$('.accountautologoff').length) {

        $('.content').append(errorMessage);

        $('body').addClass('disabled');
        $('html').removeClass('loading');
        $('.accountautologoff').hide().fadeTo(600, 1, 'swing');
        $(document).bind('mousewheel', function (e) { e.preventDefault(); });
        $('.content-home, .content-search').fadeTo(600, 0.5, 'swing');
}
\$\endgroup\$
1
\$\begingroup\$

I'm probably not going to be the best one to help with this because of the language, but here's what I see.

You should be able to lump them together by saving the state of the account to a variable before calling the function:

If (logic to check account stutus = disabled) 
Then  AccountStatus = "disabled"
If (logic to check account status = deleted) 
Then  AccountStatus = "deleted"

Then call the function:

function accountDeletedorDisabled() 

and modify it to state the "Your account has been" AccountStatus then the rest of your function.

Note: slow posted. Didn't see other responses before I posted. Sorry.

\$\endgroup\$
1
\$\begingroup\$

I would do it like this (I did not test this):

  • Instead of calling accountDisabled, call accountChanged(false).
  • Instead of calling accountDeleted, call accountChanged(true).

function accountChanged(boolean accountDeleted) {
    if (!$('.accountautologoff').length) {
        if (accountDeleted)
            $('.content').append('<div class="accountautologoff"><span>Your account has been removed</span><br /><span>Contact the system administrator</span></div>');
        else
            $('.content').append('<div class="accountautologoff"><span>Your account has been disabled</span><br /><span>Contact the system administrator</span></div>');


        $('body').addClass('disabled');
        $('html').removeClass('loading');
        $('.accountautologoff').hide().fadeTo(600, 1, 'swing');
        $(document).bind('mousewheel', function (e) { e.preventDefault(); });
        $('.content-home, .content-search').fadeTo(600, 0.5, 'swing');
};
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy