1
\$\begingroup\$

I am reading a book call Front end Web Development "The big nerd ranch guide", in which I came across a silver challenge:

Go to your favorite search engine and search for "anything".

Open the DevTools to the console.

With the functions you wrote in this chapter as a reference, attach event listeners to all of the links and disable their default click functionality.

Here's what I wrote:

function selectLink() {
    'use strict'
    var links = document.querySelectorAll('a');
    var linksArray = [].slice.call(links);
    return linksArray;
}

function disableClick (dis) {
    'user strict';
    dis.addEventListener('click', function (event) {
        event.preventDefault();
    });
}

function click() {
    'use strict';
    var link = selectLink();
    link.forEach(disableClick);
} 

click();

I don't know whether this is the correct method to do it, since there is no solution on the book for it.

Can I do better?

\$\endgroup\$
  • \$\begingroup\$ Sorry but we review code, the goal of this site isn't to explain how code works. \$\endgroup\$ – IEatBagels May 29 at 16:26
  • \$\begingroup\$ There is no correct way, if it works, then it works. You could just do addEventListener("click",e=>e.composedPath().some(el=>el.tagName=="A") && e.preventDefault()); Which checks to see if an anchor is involved via the global this. There are many more ways it can be done, which is correct who knows??? \$\endgroup\$ – Blindman67 May 29 at 16:43
  • 2
    \$\begingroup\$ Mini review: I expect 'user strict' is meant to be 'use strict'. \$\endgroup\$ – Toby Speight May 30 at 11:59
5
\$\begingroup\$

Refactoring is commonly seen as "moving out repeating code into reusable functions". But it can also easily mean "moving code that's only ever used once back to where it's being used". Although the book led you to writing individual functions, you don't have to split all this code up.

Also, if you're just turning a NodeList into an array just to use array.forEach(), then this conversion is unnecessary. NodeList already comes with a forEach method (unless you still care about IE).

Your code could be as short as:

document.querySelectorAll('a').forEach(link => {
  link.addEventListener('click', event => event.preventDefault())
})
\$\endgroup\$

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.