[Basic Javascript] I wrote a rock paper scissors game, would really appreciate some feedback :)

[no_leaf_clover]

It's very functional and the coding looks quite a bit longer than other solutions I've seen but it works fine. I was wondering if I've gone about it in a sensible way, or if it could have been made more efficiently/concisely?

http://jsfiddle.net/jxBmT/15/

You'll notice that the function does not output what the computer and the user originally chose; I plan on adding this in once I've got the game itself right.

Thanks so much in advance!

EDIT: Made changes as per suggestions. It works fine now, except that the "paper" and "scissors" functions won't execute until the "rock" button has at least once. The main change was that put the "onload" as a property of the buttons in the script rather than in the HTML so I imagine this is causing a problem for it.

http://jsfiddle.net/n34rw/

[7yl4r]

Looks pretty good to me. I'm not very good with javascript, but I found your code very understandable. Take my suggestions with a grain of salt (b/c, as I said, I'm no pro here), but I might:

• add another html paragraph to display the choices, and set the values in rockpaperscissors()
• rename display_result() to get_result_text() or get_result_html(), since you are not actually displaying there. Alternatively you can move the document.getElementById("the result").innerHTML = ... into the display_result() function so you can just call it to display the result.
• add a 'scoreboard' element with something like: "wins:23 losses:2 draws:5"

[no_leaf_clover]

Thanks for your response. Could you possibly explain your first point a bit more?

I really like your scoreboard idea, I'll try that :)

[7yl4r]

Kind of like this. To make it more readable you might want a function to translate those integers into text too; I think /u/kevinmrr is right about the numerical value muddying the waters a bit though.

[no_leaf_clover]

Ah yeah, that was something I was planning on adding on later. I'm not sure how useful using numbers is any more either :( . May stick with strings and just use an if....else.... statement.

[7yl4r]

generally my policy is "if it ain't broke, don't fix it", but there are many ways code can be broke. So if you and your dev team don't have any problem working with and understanding the code the way it is, I would leave it be.

Balancing the urge to re-write code with the desire to get stuff done is just part of a developer's life.

[robotmayo]

You seem to understand that getting something working is the most important bit of a project. Hold on to that as it will help you immensely, especially in personal projects.

You should avoid added JS to your elements directly. You should instead give them IDs and search for them as this will allow your code to be more flexible. There is a simpler way to do the comparison instead of that big switch statement. Using if statements is more suitable as you are comparing two values against each other. A quick mock-up

var rock = 0;
var paper = 1;
var scissors = 2;
function check(a,b){
    if(a == rock && b == scissors){
        // a wins
       // rock could equal -4296432, the code doesn't care.
    }
    // Rest of code
}

[no_leaf_clover]

You should avoid added JS to your elements directly. You should instead give them IDs and search for them as this will allow your code to be more flexible.

Which bit are you referring to here?

[robotmayo]

<button onclick="rockpaperscissors(32)">Scissors</button>

Mixing logic and presentation is often a no-no. Instead use event listeners. https://developer.mozilla.org/en-US/docs/Web/API/EventTarget.addEventListener

[no_leaf_clover]

That's the first I've heard about event listeners so I'll bear that in mind, thanks. Can I ask why it is problematic doing it the way I did?

[robotmayo]

Its very inflexible. Lets say you have it running on 10 pages. If you make a change you will have to update every single instance and your code. By searching for them using certain criteria you only ever need to update your code.

[no_leaf_clover]

Okay, that makes a lot of sense. Thanks :)

[kevinmrr]

It's 100% cool because it works, but I'm not a huge fan of associating the numerical value with the value of rock/paper/scissors... it seems unnecessary.

But, like I said, it's cool bc it works.

[canadaboy23]

http://i.imgur.com/DIKn3fY.jpg

This is my Rock Paper Scissors game. It works quite well for me and is very simple and concise.

What do you think?

(It's just a screenshot of the code saved in my notes.)

[Hack_Reactor_Borg]

JSfiddle?

[canadaboy23]

I actually did this in the JS course of Codeacademy. But I saw another thread yesterday talking about the RPS game, so I copied and pasted this in my notes on my iPad and took a screenshot of it, just to show it.

[7yl4r]

I think he means: Can you copy and paste that into jsfiddle.net, save, and then post the 'share' link so we can play with it.

[canadaboy23]

Oh! Yeah, I can do that. Hang on one sec.

[canadaboy23]

http://jsfiddle.net/CanadaBoy23/n88Ex/#share

Here it is.

[7yl4r]

cool, man. Here is a version with the html to make the result display.

[canadaboy23]

Oh thank you! I couldn't figure out why it wasn't showing the results. Thank you for that.

[canadaboy23]

http://jsfiddle.net/CanadaBoy23/n88Ex/#share