Create a triangle arrow block dynamically

Question

I wrote a function to create one of those CSS triangle blocks and insert it right after one of the already created blocks.

function createArrow() {
    var arrowDown = {
        parent: document.querySelector('.landingItem'),
        div: document.createElement('div'),
        initDiv: function() {
            this.div.classList.add('arrow-down');
            this.div.style.borderLeftWidth = parseInt(document.documentElement.clientWidth)/2 + 'px';
            this.div.style.borderRightWidth = parseInt(document.documentElement.clientWidth)/2 + 'px';
            this.parent.appendChild(this.div);
        },
        checkDiv: function () {
            if(this.parent.querySelector('.arrow-down')) {
                this.parent.removeChild(this.parent.querySelector('.arrow-down'));
            }
            this.initDiv();
        }

    };

    arrowDown.checkDiv();
}

createArrow();
window.addEventListener('resize', createArrow);

.arrow-down {
  margin-top: -2px;
  border-top: 60px solid #501a70;
  border-left: 160px solid transparent;
  border-right: 160px solid transparent;
}

<div class="landingItem"></div>

What can I improve?

Answer 1

Is it an utility function or a module?

You named your function createArrow() and it contains an object with methods and at the end you just call one.

I think this is a bit of over engineered for such piece of code.

The first improvement I suggest you is to remove the plain object and method, and refactor the function to do stright forward the task.

Names are all

Please pay attention to the names you give to the functions/method.

When I use some kind of module or object, it is natural to me to call the initializer as the first thing.

But looking at your code, it would be the wrong way.

You just called a checkDiv().

Does is not just bad because it is unaspected, but aslo because if I look at your code, you didn't check the div!

    checkDiv: function () {
        if(this.parent.querySelector('.arrow-down')) {
            this.parent.removeChild(this.parent.querySelector('.arrow-down'));
        }
        this.initDiv();
    }

Here you search an element in the portion of the DOM tree of the this.parent, and if you find, you remove!

A check should be a safe action, not something that remove things.

I suggest you to call such method: reset()

This is more near to the task, and tell to other developer that if they call it they are going to loose what they have now.

Another thing is to remove from the method the last line:

 this.initDiv();

You should call reset() at the beginning of the init(), not the init() at the end of the reset().

And yes, about names: you don't need the Div part in the name in this case.

So, I would refactor your code as follow:

function createDownArrow() {
    const parent = document.querySelector('.landingItem');

    var arrow = parent.querySelector('.arrow-down'); 
    if (arrow) {
        parent.removeChild(arrow);
    }

    const element = document.createElement('div');

    element.classList.add('arrow-down');
    const borderWidth = parseInt(document.documentElement.clientWidth)/2;

    element.style.borderLeftWidth = borderWidth + 'px';
    element.style.borderRightWith = borderWidth + 'px';

    parent.appendChild(element);
}

window.addEventListener("load", createDownArrow);
window.addEventListener("reset", createDownArrow);

.arrow-down {
  margin-top: -2px;
  border-top: 60px solid #501a70;
  border-left: 160px solid transparent;
  border-right: 160px solid transparent;
}

<div class="landingItem"></div>