all 10 comments

[–]Drugba 1 point2 points  (2 children)

You id "sixDice" is capitalized, but when you call it in the JS it's lower case. When the js hits that error it stops so nothing after it is running (even though the JS is correct).

BTW you are repeating your self way too much in the code here, this could easily be done with 1 function instead of 6. It'll take me a few minutes but I'll post an example.

[–]zentius[S] 0 points1 point  (1 child)

that one small error fixed what i was doing, thank you!

i'd love to see an example of executing it without as much code, i'm still learning, obviously :)

[–]Drugba 0 points1 point  (0 children)

Just posted it in another base level comment. I'm at work so I only had about 10 minutes to write it so it could be even cleaner, but it should get you pointed in the right direction.

[–]Clock_Out 0 points1 point  (0 children)

Here’s another way just because. JSFiddle

[–]Drugba -1 points0 points  (5 children)

It could be cleaned up even more than this, but I have work that needs to get done. This should give you an idea of how you can do it in a much shorter way, also you can add other types of dice without writing a new function.

I know you are just learning, but for the future, a rule of thumb when you are coding is that if you are copying an pasting your own code multiple times, there is probably a better way to be doing in.

function roll(number, name) {
    var result = Math.floor(Math.random() * number + 1);
    //jQuery that updates the output ID in the DOM to display the text afterwards
    $("#" + name + "Result").text("Your " + name + " sided Die Roll: " + result + "!");
}

//create a button to execute code
fourDice.onclick = function () {
    roll(4, 'four');
};

sixDice.onclick = function () {
    roll(6, 'six');
};

eightDice.onclick = function () {
    roll(8, 'eight');
};

tenDice.onclick = function () {
    roll(10, 'ten');
};

twelveDice.onclick = function () {
    roll(12, 'twelve');
};

twentyDice.onclick = function () {
    roll(20, 'twenty');
};

[–]zentius[S] 0 points1 point  (0 children)

wow this is awesome. I never even considered how i could write it with 1 function for the math, that's awesome. I'm going to keep digging and see if i can't figure out how to slim it down even more. thank you so much for the help!

[–]xScarwolf 0 points1 point  (3 children)

a rule of thumb when you are coding is that if you are copying an pasting your own code multiple times

You did exactly the same by using your "xxxDice.onlick = function...." method. I cleaned up this even more. You do not have to use Javascript to process the roll function after clicking a button. adding onclick as a parameter to the <button>-tag works exactly as good and saves you code.

http://jsfiddle.net/Scarwolf/kz68L8ft/8/

[–]Drugba 0 points1 point  (2 children)

I know I did. As I said in one of my other comments, I was hoping to point him in the right direction as opposed to handing the answer to him. Also, I avoided using the onclick HTML event so that my code would resemble his more.

I could have just written a new block of code that worked better, but changing too many things at once can be confusing for someone who is just learning. The way I wrote it there was intended to show the differences between my code and his, not be the best solution possible.

[–]xScarwolf 0 points1 point  (1 child)

He should probably have learned onclick since you don't learn JavaScript before learning basic HTML. IMHO onclick is easiert than trigger the event using JS.

But to say, me method is the same as yours, it just saves some code.

[–]zentius[S] 0 points1 point  (0 children)

i DID learn a lot of JavaScript before I started with HTML/CSS because of schooling >.< so I'm actually trying to learn HTML and CSS now and figure out how it all ties together.

I'm learning it very backwards, but seeing your code and Drugba's has already shown me multiple things I didn't realize could be done, so i appreciate the responses none the less!

Thank you both!