3
\$\begingroup\$

I've two similar functions and I want to use one generic function with two params, dialog and buttons. How do I do it?

My functions:

LanguageLogosView.showConfirmRemoveDialog = function () {
    var def = $.Deferred();
    var buttons = {
        buttons: {
            "Remove": function () {
                def.resolve(true);
                $(this).dialog("close");
            },
            Cancel: function () {
                def.resolve(false);
                $(this).dialog("close");
            }
        }
    };
    $('#dialog-confirm-remove').dialog($.extend(LanguageLogosView.dialogConfirmDefaultSettings, buttons));
    return def;
}

LanguageLogosView.showConfirmEmptyRowsDialog = function () {
    var def = $.Deferred();
    var buttons = {
        buttons: {
            "Save": function () {
                def.resolve(true);
                $(this).dialog("close");
            },
            Cancel: function () {
                def.resolve(false);
                $(this).dialog("close");
            }
        }
    };
    $("#dialog-confirm-save-with-empty-rows").dialog($.extend(LanguageLogosView.dialogConfirmDefaultSettings, buttons));
    return def;
}
\$\endgroup\$
3
\$\begingroup\$

First of all, always return def.promise() - not def itself. If you return the Deferred object itself, other code can call resolve/reject on it. So keep def to yourself, and only return its promise.

Also, why not use reject() for the Cancel buttons instead of resolve(false)? If you use reject, it'll probably make the rest of your code simpler, since you don't need to check the resolve arguments. Your "success" handler either gets called, or it doesn't.

Second, since you're building buttons in both cases, that'd probably be the part I'd extract. For instance

function makeDialogButtons(resolveTitle, rejectTitle) {
  var def = $.Deferred(),
      buttons = {};

  buttons[resolveTitle] = function () {
    def.resolve();
    $(this).dialog('close');
  };

  buttons[rejectTitle] = function () {
    def.reject();
    $(this).dialog('close');
  };

  return { buttons: buttons, promise: def.promise() };
}

And use it like so

LanguageLogosView.showConfirmEmptyRowsDialog = function () {
  var buttons = makeDialogButtons('Save', 'Cancel');
      options = $.extend(LanguageLogosView.dialogConfirmDefaultSettings, buttons.buttons);
  $("#dialog-confirm-save-with-empty-rows").dialog(options);
  return buttons.promise;
};

It's not great, but it should do the job.

\$\endgroup\$
  • \$\begingroup\$ Thanks about returning promise, I've fixed my code already. \$\endgroup\$ – Viktor Jun 2 '14 at 15:56
  • \$\begingroup\$ To @nrw who suggested an edit that got rejected: You were right. Unfortunately, the edit was rejected just 1 minute before I saw it, otherwise I would've approved it. \$\endgroup\$ – Flambino Jun 2 '14 at 16:00

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.