6
\$\begingroup\$

To get the book list from https://www.java67.com/2015/09/top-10-algorithm-books-every-programmer-read-learn.html , I am using the following code in the console of the firefox DevTools:

var select =  document.querySelectorAll("div.post-body.entry-content div h3 b");

for (i = 0; i < select.length; ++i) {

  try {
    var title =  select[i].querySelectorAll("u")[0];
    console.log (title.innerHTML);
  }
  catch(err) {
  }

  try {
    var author =  select[i].querySelectorAll("a")[0];
    console.log (author.innerHTML);
  }
  catch(err) {
  }

}

The output is:

Introduction to Algorithms by Thomas H. Corman debugger eval code:12:13
Algorithms by Robert Sedgewick &amp; Kevin Wayne debugger eval code:12:13
The Algorithm Design Manual by Steve S. Skiena debugger eval code:12:13
Algorithm for Interviews debugger eval code:12:13
5. Algorithm in Nutshell debugger eval code:5:13
6. Algorithm Design by Kleinberg &amp; Tardos debugger eval code:5:13
<a href="https://dev.to/javinpaul/10-best-books-to-learn-data-structure-and-algorithms-in-java-python-c-and-c-5743" target="_blank">7. Introduction to Algorithms: A Creative Approach</a> debugger eval code:5:13
7. Introduction to Algorithms: A Creative Approach debugger eval code:12:13
8. The Design and Analysis of Algorithms debugger eval code:5:13
9. Data Structures and Algorithms. Aho, Ullman &amp; Hopcroft debugger eval code:5:13
10. Python Algorithms: Mastering Basic Algorithms in the Python Language debugger eval code:5:13
undefined  

You can see from the code that the first part div.post-body.entry-content div h3 b is same. Then I am using u or a. Is there any easier way to achieve this.

\$\endgroup\$
8
\$\begingroup\$

Don't implicitly create global variables for (i = 0; uses a global i. Always declare variables before using them - in modern times, declare them with let or const, not var (which has too many problems, such as an unintuitive function scope rather than block scope, to be worth using nowadays)

Iterators Or, even better, rather than iterating manually, invoke the NodeList's iterator, and use for..of instead, eg:

for (const b of bs) {
  // do stuff with each <b> element
}

querySelector If you only want to select one element matching a selector, don't use querySelectorAll followed by [0] index lookup - use querySelector instead.

try/catch In predictable, synchronous code, there shouldn't be any need for try/catch. If you aren't sure whether an element exists, just test it first, instead of possibly throwing an error.

textContent Use textContent rather than innerHTML - textContent is faster and more appropriate when you aren't deliberately retrieving HTML markup. Also, .innerHTML gives you difficult HTML entities, as you can see with your Kleinberg &amp; Tardos, instead of Kleinberg & Tardos

Try something like this instead:

const bs = document.querySelectorAll("div.post-body.entry-content div h3 b");
for (const b of bs) {
  const child = b.querySelector('u, a');
  if (child) {
    console.log(child.textContent);
  }
}
\$\endgroup\$
3
  • \$\begingroup\$ And that's called a code review. Thank you very much. \$\endgroup\$ – blueray Sep 22 '20 at 16:46
  • 1
    \$\begingroup\$ @blueray Also, empty catch blocks are bad. You should at least log the error so you know it's happening. \$\endgroup\$ – marcellothearcane Sep 23 '20 at 10:28
  • 1
    \$\begingroup\$ @marcellothearcane I believe the try/catch was only to make sure that when a b doesn't have a u or a child, the program continues instead of throwing an error and stopping. In other words, it was just there for control flow (and checking whether the element exists first before trying to examine its content is the better approach) \$\endgroup\$ – CertainPerformance Sep 23 '20 at 14:14
3
\$\begingroup\$

In a CSS selector you can use a comma to select multiple items.

var select =  document.querySelectorAll("div.post-body.entry-content div h3 b");

for (i = 0; i < select.length; ++i) {
  var title =  select[i].querySelectorAll("u, a")[0];
  console.log (title.innerHTML);
}

This will get both u and a elements, but if only one of them exists, which seems to be the case here, it will get what you need.

You can even use a single querySelectorAll, but you have to repeat the whole thing.

document.querySelectorAll("div.post-body.entry-content div h3 b a, div.post-body.entry-content div h3 b u")
\$\endgroup\$
0
3
\$\begingroup\$

Spacing

While spacing doesn't matter in JavaScript, some lines have more space than needed. For example, on this line, there are two spaces after the assignment operator:

var select =  document.querySelectorAll("div.post-body.entry-content div h3 b");
             ^

One space in that spot would be fine. Technically no space is required but idiomatic code contains a single space in such spots. Some style guides call for a single space as well (e.g. Google JS).

And the calls to function console.log don't need a space before the opening parenthesis:

 console.log (author.innerHTML);
            ^

Naming

The variable name select might be more appropriate as selection or elements so as to avoid confusion with a <select> element.

Names like title and author could be confused for strings yet those variables hold references to HTMLElement nodes so some could argue that having Element or El in the name would be appropriate.

\$\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.