This is jQuery code which works like this: depending on a user choice it changes the color and background of some elements in the DOM.
The behavior is ok, is doing exactly what I want.
And as you may see, there is some code repeated. What's the best way to keep a good performance by doing a function in order to avoid repeating code?
function ArticleQuiz($el){
this.$el = $el;
this.answer = this.$el.data('answer');
this.answerContainer = this.$el.find('.answer-container');
this.result = this.$el.find('.result');
this.correctAnswer = this.$el.find('.correct-answer');
this.incorrectAnswer = this.$el.find('.incorrect-answer');
this.quizLogic();
return this;
}
ArticleQuiz.prototype.quizLogic = function(){
var THIS = this;
var answer;
$('.masthead-article-quiz').parent().css('background', '#eee');
$('.quiz-cols a', this.$el).click(function(e) {
THIS.target = $(e.target);
e.preventDefault();
// exit if choice already made: users can't change their pick
if ($(this).parents('.quiz-cols').parent().find('.white-font').length) return;
// set class according to data-answer:
if (THIS.target.hasClass('icon-false-shape')) {
THIS.answerContainer.show();
$(this).addClass('background-' + ((THIS.answer === false) ? 'green' : 'red')).addClass('white-font');
THIS.result.addClass('font-' + ((THIS.answer === false) ? 'green' : 'red'));
answer = THIS.answer ? THIS.incorrectAnswer.show() : THIS.correctAnswer.show();
}
if (THIS.target.hasClass('icon-true-shape')) {
THIS.answerContainer.show();
$(this).addClass('background-' + ((THIS.answer === true) ? 'green' : 'red')).addClass('white-font');
THIS.result.addClass('font-' + ((THIS.answer === true) ? 'green' : 'red'));
answer = !THIS.answer ? THIS.incorrectAnswer.show() : THIS.correctAnswer.show();
}
});
return THIS;
};
2 Answers 2
The first thing that jumps out at me is the tight coupling it has with the current application in which it is being used. Why hard code all those CSS selectors? This class really cries out for configuration.
You might see a pattern like this used to achieve decoupling of class from config.
function ArticleQuiz($el, $config){
this.config = {};
...
init() {
$.extend(this.config, this.defaultConfig, $config || {});
...
}
this.init();
}
ArticleQuiz.defaultConfig = {
answerDataLabel = 'answer',
answerContainerSelector = '.answer-container',
...
}
You should consider taking the approach of using meaningful class names to apply multiple CSS styles at once. So, rather than using specific semantic color and font class names, define a CSS class that encapsulates the entire state of being a correct/wrong answer. So taking this approach along with the suggestion to store configuration something might allow you to change this:
$(this).addClass('background-' + ((THIS.answer === false) ? 'green' : 'red')).addClass('white-font');
THIS.result.addClass('font-' + ((THIS.answer === false) ? 'green' : 'red'));
answer = THIS.answer ? THIS.incorrectAnswer.show() : THIS.correctAnswer.show();
Into something like:
var classToApply = THIS.config.correctAnswerClass;
if(THIS.answer === true) {
classToApply = THIS.config.incorrectAnswerClass;
}
$(this).addClass(classToApply).show();
Your QuizLogic
function would not seem to have reason to be in the static (prototype) context, as you are using it as a constructor for your class. I would think this function should be within class function scope instead on prototype and should be call something like init()
or similar common name for constructors in javascript. Note how I present the init()
function in my first code example above.
Consider first populating your instance properties in the constructor rather than in the main class code. This gives you the ability to do things such as validate that the selectors do not return empty collections and similar activities needed to put the object in the proper state for use. If these validations fail, you could throw an error to prevent the object from being used in an improper state.
You can use angular directives, for example:
<input type="button" value="set color" ng-click="myStyle={color:'red'}">
<input type="button" value="set background" ng-click="myStyle={'background-color':'blue'}">
<input type="button" value="clear" ng-click="myStyle={}">
<br/>
<span ng-style="myStyle">Sample Text</span>
<pre>myStyle={{myStyle}}</pre>
correctAnswer
andincorrectAnswer
defined? \$\endgroup\$