CODEWITHSUNDEEP
javascript
b134692
b134692

Reputation: 93

combining two javascript functions

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();
}

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

Upvotes: 0

Views: 175

Answers (2)

mavrosxristoforos
mavrosxristoforos

Reputation: 3663

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 || "";)

Upvotes: 1

loxxy
loxxy

Reputation: 13151

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.

Upvotes: 2

Related Questions