HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Before/after jQuery highlighter

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
afterbeforehighlighterjquery

Problem

I have a webpage that shows shorthand code on the left, and its long form on the right. When you hover over code on the left (signified with a space-separated array in the data-from attribute), it uses jQuery to highlight corresponding code on the right (signified with a space-separated array in the data-to attribute), and vice-versa. It's working great, but it feels very long.

Script

`function containsAny(haystack, needles) {
for(needle in needles)
if (haystack.indexOf(needles[needle]) >= 0)
return true;
}
$(document).ready(function() {
/ from to /
$("[data-from]").each(function(i, e) {
var $e = $(e),
froms = $e.data("from").toString().split(/\s+/); // must do toString in case we get an int
$e.hover(function() {
$e.addClass("hilighted");
$("[data-to]").each(function(i2, e2) {
var $e2 = $(e2),
tos = $e2.data("to").toString().split(/\s+/);
if (containsAny(tos, froms))
$e2.addClass("hilighted");
});
},
function() {
$("[data-to]").each(function(i2, e2) {
$e.removeClass("hilighted");
var $e2 = $(e2),
tos = $e2.data("to").toString().split(/\s+/);
if (containsAny(tos, froms))
$e2.removeClass("hilighted");
});
});
});

/ to from /
$("[data-to]").each(function(i, e) {
var $e = $(e),
tos = $e.data("to").toString().split(/\s+/);
$e.hover(function() {
$e.addClass("hilighted");
$("[data-from]").each(function(i2, e2) {
var $e2 = $(e2),
froms = $e2.data("from").toString().split(/\s+/);
if (containsAny(froms, tos))
$e2.addClass("hilighted");
});
},
function() {
$("[data-from]").each(function(i2, e2) {
$e

Solution

Here's your code, and I've pointed out some potential problems:

for(needle in needles)
    if (haystack.indexOf(needles[needle]) >= 0)
        return true;


While this is possible, I suggest you use {} instead. This avoids ambiguity, and future danger, especially when you'll be adding more functionality beyond just a single line.

Additionally, for-in runs over prototype properties. If you happen to have a property that is similarly named as a native property in the prototype, you'll be in trouble. To avoid that, check using hasOwnProperty before proceeding with the code.

$(document).ready(function() {


Probably not better, but there's a shorthand for this: $(function(){...})

$("[data-from]").each(function(i, e) {


I'd probably avoid using attribute selectors. They're known to be slow, depending on the parser implementation. I suggest you stick to classes.

froms = $e.data("from").toString().split(/\s+/); // must do toString in case


A shorthand to convert stuff to string is to do '' + to it. Not so verbose, but it's somewhat elegant and short.

Code Snippets

for(needle in needles)
    if (haystack.indexOf(needles[needle]) >= 0)
        return true;
$(document).ready(function() {
$("[data-from]").each(function(i, e) {
froms = $e.data("from").toString().split(/\s+/); // must do toString in case

Context

StackExchange Code Review Q#70111, answer score: 2

Revisions (0)

No revisions yet.