Welcome to the Treehouse Community
Want to collaborate on code errors? Have bugs you need feedback on? Looking for an extra set of eyes on your latest project? Get support with fellow developers, designers, and programmers of all backgrounds and skill levels here with the Treehouse Community! While you're at it, check out some resources Treehouse students have shared here.
Looking to learn something new?
Treehouse offers a seven day free trial for new students. Get access to thousands of hours of content and join thousands of Treehouse students and alumni in the community today.
Start your free trial
Brian Patterson
19,588 PointsHelp needed in refactoring
Hello, wonderful Treehouse Community. I just need some help with refactoring some javaScript. The app is a small Rock Paper Scissors Web App. I have three functions for Win, Lose or Draw. I have noticed that it is not very "DRY".
function win(userChoice, computerChoice) {
const smallUserWord = "user".fontcolor("red").fontsize(3).sub();
const smallCompWord = "comp".fontcolor("blue").fontsize(3).sub();
const userChoice_div = document.getElementById(userChoice);
userScore++;
userScore_span.innerHTML = userScore;
computerScore_span.innerHTML = computerScore;
result_p.innerHTML = `${convertToWord(userChoice)}${smallUserWord} beats ${convertToWord(computerChoice)}${smallCompWord} You win!`;
userChoice_div.classList.add('green-glow');
setTimeout(() => userChoice_div.classList.remove('green-glow'), 300);
}
function lose(userChoice, computerChoice) {
const smallUserWord = "user".fontcolor("red").fontsize(3).sub()
const smallCompWord = "comp".fontcolor("blue").fontsize(3).sub()
const userChoice_div = document.getElementById(userChoice);
computerScore++;
userScore_span.innerHTML = userScore;
computerScore_span.innerHTML = computerScore;
result_p.innerHTML = `${convertToWord(userChoice)}${smallUserWord} loses ${convertToWord(computerChoice)}${smallCompWord} You lost!`;
userChoice_div.classList.add('red-glow');
setTimeout(() => userChoice_div.classList.remove('red-glow'), 300);
}
function draw(userChoice, computerChoice) {
const smallUserWord = "user".fontcolor("red").fontsize(3).sub();
const smallCompWord = "comp".fontcolor("blue").fontsize(3).sub();
const userChoice_div = document.getElementById(userChoice);
result_p.innerHTML = `${convertToWord(userChoice)}${smallUserWord} equal ${convertToWord(computerChoice)}${smallCompWord} Its a draw`;
userChoice_div.classList.add('gray-glow');
setTimeout(() => userChoice_div.classList.remove('gray-glow'), 300);
}
I wonder if I put the following in a seperate function.
function drawColor () {
const smallUserWord = "user".fontcolor("red").fontsize(3).sub();
const smallCompWord = "comp".fontcolor("blue").fontsize(3).sub();
const userChoice_div = document.getElementById(userChoice);
return userChoice_div;
}
Just want to know how would I incorporate it into the Win, Lose and Draw functions. Any help would be appreciated.
Brian Patterson
19,588 PointsHi Jennifer Nordell here is a link to the repo on Github. rock paper scissors
1 Answer
Steven Parker
243,228 PointsHaving the function set local variables that are not otherwise used is not helpful. But you could have the shared function do even more by passing it the arguments and the name of the class to add and remove.
But be aware that you're using several functions that are not recommended for production. The MDN pages have this to say about "fontcolor", "fontsize" and "sub":
Deprecated
This feature has been removed from the Web standards. Though some browsers may still support it, it is in the process of being dropped. Avoid using it and update existing code if possible. Be aware that this feature may cease to work at any time.
For a fun addition, you might consider providing an option to play the "rock-paper-scissors-lizard-spock" variation.
Brian Patterson
19,588 PointsMaybe I didn't word it correctly. How would you include that function within the other functions?
Steven Parker
243,228 PointsWhat I was hinting at before is that the function will need some modification before it can be included in the other functions in a way that will be useful. In particular, setting values for "smallUserWord" and "smallCompWord" will not be helpful unless the common function also does something with them.
My suggestion is to pass the arguments "userChoice" and "computerChoice" to the common function, along with the class name, and let the common function do all the work except for changing the score.
Brian Patterson
19,588 PointsHi Steven Parker thanks for the reply you have been really helpful. I need a bit of help setting this up.
function drawColor (userChoice, computerChoice) {
//I'm lost to what to do.
}
Steven Parker
243,228 PointsYou'll need to pass more arguments. In addition to these, you'll need the name of the class and the message text to be shown.
Then the body of the function will be much like what you already have for "draw", except for substituting the parameters for the class names and message.
Brian Patterson
19,588 PointsI don't have a class and I am still confused on what to do?
Steven Parker
243,228 PointsLooking at the original question code, each function has a class associated with it which needs to be added and then removed using a timer. In "win", the class is "green-glow"; in "lose" it is "red-glow", and in "draw" it is "gray-glow".
So after converting the code in "draw" into the new "drawColor", the "draw" function can be compacted to just a single call:
function draw(userChoice, computerChoice) {
draw(userChoice, computerChoice, "It's a draw", "gray-glow");
}
The others will be similar but will also update the score.
Jennifer Nordell
Treehouse TeacherJennifer Nordell
Treehouse TeacherThis might be easier if you posted a link to the repo or a snapshot to the workspace so that we can see from where these functions are being called and the DOM elements being selected.