0

Below is the code fragment, there is something which I don't like:

function insert(el, child, before){
  if ( before ) {
    el.insertBefore(child, el.childNodes[0]);
  } else {
    el.appendChild(child);
  }
}

Why not instead have two separate methods like insertBefore and insertAfter? what are the pros and cons of this and other approach?

Update:

I got this nice article explaining what I wanted.

Mike Stockdale
  • 5,256
  • 3
  • 29
  • 33
CodeYogi
  • 1,352
  • 1
  • 18
  • 41
  • What's the purpose of this question? You have a code you don't like?? Change it! – Amit May 02 '16 at 18:32
  • @Amit the purpose of this question is writing better code. The definition of better being easy to understand and test. I think question together with tags clarify the intent properly. – CodeYogi May 02 '16 at 18:44
  • What's the purpose of this function? why don't you simply use appendChild, for example? Is this part of some lib? – Thomas May 02 '16 at 18:51
  • @Thomas yes, its a public lib [here](https://github.com/kenwheeler/cash/blob/master/src/manipulation.js). – CodeYogi May 02 '16 at 18:52

1 Answers1

2

The whole purpose of this function is to avoid having to put an if statement in all the places where you call the function. So if you have lots of places that look like:

if (something) {
    foo.insertBefore(bar, foo.childNodes[0]));
} else {
    foo.appendChild(bar);
}

You can simplify all of them to:

insert(foo, bar, something);

With your two methods, it would become:

if (something) {
    insertBefore(foo, bar);
} else {
    insertAfter(foo, bar);
}

which isn't too much better than the original.

Barmar
  • 741,623
  • 53
  • 500
  • 612
  • But it requires client code to remember to pass that flag each time it calls that method, moreover I would know whether I have to insert before or after in my client code no need to have if conditions scattered throughout my code. – CodeYogi May 03 '16 at 03:36
  • You would only use this method when the choice needs to be determined dynamically. It's like why jQuery has both `hide`, `show`, and `toggle`. You use `toggle` when it's dynamic, and `hide/show` when it's known a priori. – Barmar May 03 '16 at 04:15