combining two javascript functions

Question

Are there any benefits of combining the below functions?

window.onload = function() {
  if (localStorage.getItem("txt")) {
    textarea.value = localStorage.getItem("txt");
    changeDocTitle(localStorage.getItem("filename"));
    isModified = true;
  } else {
    changeDocTitle(untitled);
  }
};

function newNote() {
  if (!isModified || dontSave()) {
    textarea.value = "";
    changeDocTitle(untitled);
  }
  textarea.focus();
}

After combining it will look like this:

window.onload = function() {
  if (localStorage.getItem("txt")) {
    newNote(localStorage.getItem("txt"), localStorage.getItem("filename"));
  } else {
    newNote();
  }
};

function newNote(txt, filename) {
  if (!isModified || dontSave()) {
    textarea.value = txt || "";
    changeDocTitle(filename || untitled);
    if (textarea.value) {
      isModified = true;
    }
  }
  textarea.focus();
}

I'll call the newNote() function with keyboard shortcuts too..

What is the difference between the two, and is there any reason to prefer one over the other?

Community · Accepted Answer · 2017-04-13 12:40:33Z

2

The second one.

If the scope of newNote is limited to the onload function, then there is no reason to dirty the global scope.

EDIT

Frankly, it doesn't matter much. If you binding the keyboard event, then the function will remain in scope throughout the document scope. So either would be good.

Besides, questions in SO, are more problem oriented. Try https://codereview.stackexchange.com/, instead.

edited Apr 13, 2017 at 12:40

CommunityBot

11 silver badge

answered Oct 24, 2013 at 5:46

loxxy

13.2k2 gold badges28 silver badges57 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

mavrosxristoforos · Accepted Answer · 2013-10-24 06:04:58Z

This may be primarily opinion-based, but I would go with the second one.
Reasons:

Increased Readability and Easier Maintainance

It's obviously easier to read and maintain one function instead of two, and you keep your code in one place altogether.

No waste of resources

You create the variables you need in your function and they go away when you are done with it.

Warning, though.

You should know that there are certain things to avoid. For example, if your newNote function will require more and more arguments over time, you may consider changing the scope, and possibly maintain it out of the function to avoid having a function with (let's say) a dozen arguments, because you just lose the benefits listed above.

P.S.: I think the second code you described has some errors. Unless you just wrote it like that to explain what you intend to do. (I'm talking about textarea.value = txt || "";)

Collectives™ on Stack Overflow

combining two javascript functions

2 Answers 2

Comments

Increased Readability and Easier Maintainance

No waste of resources

Warning, though.

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Increased Readability and Easier Maintainance

No waste of resources

Warning, though.

Comments

Your Answer

Sign up or log in

Post as a guest

Related