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

jQuery word quiz using local storage

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

Problem

Is there any way to reduce this?

```
//vars
defaultTextColor="#444"
correct = "#99CC00"
incorrect = "#FF4719"
answer1value = "1"
answer2value = "3"
totalquestions= "5"

//tap on the words
$(function() {
$("#word1a").click(function() {
$("#word1b").css("text-decoration", "line-through").css("opacity", "0.2").css("color", defaultTextColor);
$("#word1a").css("text-decoration", "none").css("opacity", "1").css("color", defaultTextColor);
$("#slash1").css("text-decoration", "line-through").css("opacity", "0.1");
saveanswers();
});

$("#word1b").click(function() {
$("#word1b").css("text-decoration", "none").css("opacity", "1").css("color", defaultTextColor);
$("#word1a").css("text-decoration", "line-through").css("opacity", "0.2").css("color", defaultTextColor);
$("#slash1").css("text-decoration", "line-through").css("opacity", "0.1")
saveanswers();
});

$("#word2a").click(function() {
$("#word2b").css("text-decoration", "line-through").css("opacity", "0.2").css("color", defaultTextColor);
$("#word2a").css("text-decoration", "none").css("opacity", "1").css("color", defaultTextColor);
$("#slash2").css("text-decoration", "line-through").css("opacity", "0.1");
saveanswers();
});

$("#word2b").click(function() {
$("#word2b").css("text-decoration", "none").css("opacity", "1").css("color", defaultTextColor);
$("#word2a").css("text-decoration", "line-through").css("opacity", "0.2").css("color", defaultTextColor);
$("#slash2").css("text-decoration", "line-through").css("opacity", "0.1")
saveanswers();
});

$("#word3a").click(function() {
$("#word3b").css("text-decoration", "line-through").css("opacity", "0.2").css("color", defaultTextColor);
$("#word3a").css("text-decoration", "none")

Solution

-
You should be using CSS with style sheets, not by setting all styles on the elements directly. You can switch the styles for a certain element by applying and unapplying a specific class.

-
Your naming scheme suggests that you want more complex data structures, e.g. an array of inputs, or an array of word objects like {slash: ..., a: ..., b: ...} instead of slash2, word2a, word2b. You can then cache the selectors in those fields.

For example, this piece of code

//resets the inputs on change
$(function() {
    $("#answer1").change(function() {
        {$(this).css("background-color", "lightgrey").css("color", defaultTextColor)}
    }); 
    $("#answer2").change(function() {
        {$(this).css("background-color", "lightgrey").css("color", defaultTextColor)}
    }); 
});


Could become something like

$(function() {
  for(var i = 0; i < answers.length; i++) {
    answers[i].change(function() {
      $(this).css(...);
    });
  }
});


where answers is an array of jQuery objects.

-
The same ideas hold true for your save data. Do not use variable names to impose a hierarchy, but instead complex data structures. Repetitive code like

if (word1asave=="line-through")
    {$('#word1a, #slash1').css("text-decoration", "line-through").css("opacity", "0.2")
}
...


could become

var words = savedata.words;   // [{a:String, b:String, $:{a:$, b:$, slash:$}}, ...]
for (var i = 0; i < words.length; i++) {
  var word = words[i];
  if (word.a === "line-through") {
     word.$.a    .css(...);
     word.$.slash.css(...);
  }
  if (word.b === "line-through") {
     word.$.b    .css(...);
     word.$.slash.css(...);
  }
}


I am sure you now see that this can be abstracted further.

-
Every time you use == instead of ===, a kitten dies. Granted, it isn't necessary in your code, but it's still good style.

-
I see you are leaving out many semicolons. I'd suggest to not rely on automatic semicolon insertion, and finish each line with an operator.

Code Snippets

//resets the inputs on change
$(function() {
    $("#answer1").change(function() {
        {$(this).css("background-color", "lightgrey").css("color", defaultTextColor)}
    }); 
    $("#answer2").change(function() {
        {$(this).css("background-color", "lightgrey").css("color", defaultTextColor)}
    }); 
});
$(function() {
  for(var i = 0; i < answers.length; i++) {
    answers[i].change(function() {
      $(this).css(...);
    });
  }
});
if (word1asave=="line-through")
    {$('#word1a, #slash1').css("text-decoration", "line-through").css("opacity", "0.2")
}
...
var words = savedata.words;   // [{a:String, b:String, $:{a:$, b:$, slash:$}}, ...]
for (var i = 0; i < words.length; i++) {
  var word = words[i];
  if (word.a === "line-through") {
     word.$.a    .css(...);
     word.$.slash.css(...);
  }
  if (word.b === "line-through") {
     word.$.b    .css(...);
     word.$.slash.css(...);
  }
}

Context

StackExchange Code Review Q#32873, answer score: 6

Revisions (0)

No revisions yet.