Jump to content

Randomly Changing Color


brooke_theperson

Recommended Posts

Hey, so I have a question regarding my previous post. I am trying to randomly change the color of a div just by ppressing it. Here is my code to do so:

 

My html:

 

<button onclick="changeShirtColorRandom()">Change Shirt Color Random</button>

 

My javascript:

 

function color(color){    this.color = color;}//*All colorsvar red = new color("red");var blue = new color("blue");var green = new color("green");var purple = new color("purple");var yellow= new color("yellow");var black = new color("black");//*Array of colorsvar colors = [red, blue, green, purple, yellow, black]; //*Function that chooses a random shirt colorfunction changeShirtColorRandom{    var num = Math.floor(Math.random() * colors.length);    var colors_obj = "'" + colors[num] + "'";    document.getElementById("torsoColorChange").style.backgroundColor = colors_obj;    document.getElementById("right-sleeveColorChange").style.backgroundColor = colors_obj;    document.getElementById("left-sleeveColorChange").style.backgroundColor = colors_obj;}

 

This is just the part of the code that is relevant. The problem is when I click the button nothing happens. What is wrong with my code?

Link to comment
Share on other sites

Also, these are the div's that have these id's:

 

<div id ="torsoColorChange" class ="torso"></div><div id ="left-sleeveColorChange" class ="left-sleeve"></div><div id ="right-sleeveColorChange" class ="right-sleeve"></div>
Link to comment
Share on other sites

So I'm curious how you came up with this design pattern? To me it seems pointlessly convoluted to instantiate objects that merely contain little trivial strings.

Link to comment
Share on other sites

The problem is that you're getting the stirng value of an object rather than a string containing the name of the color. The function declaration should also have parentheses after the function name.

 

Try this instead:

//*Array of colorsvar colors = ["red", "blue", "green", "purple", "yellow", "black"]; //*Function that chooses a random shirt colorfunction changeShirtColorRandom() {    var num = Math.floor(Math.random() * colors.length);    var color = colors[num];    document.getElementById("torsoColorChange").style.backgroundColor = color;    document.getElementById("right-sleeveColorChange").style.backgroundColor = color;    document.getElementById("left-sleeveColorChange").style.backgroundColor = color;}

If you want each element to have its own random color, then create one random number for each:

function changeShirtColorRandom() {    var num, color;    num = Math.floor(Math.random() * colors.length);    color = colors[num];    document.getElementById("torsoColorChange").style.backgroundColor = color;    num = Math.floor(Math.random() * colors.length);    color = colors[num];    document.getElementById("right-sleeveColorChange").style.backgroundColor = color;    num = Math.floor(Math.random() * colors.length);    color = colors[num];    document.getElementById("left-sleeveColorChange").style.backgroundColor = color;}
Link to comment
Share on other sites

Actually inglome, before I tried your thing, none of my functions are working anymore. I don't know why but now the functions that worked earlier aren't working anymore. Why might this happen? Also davej, I am not sure what your question is.

Link to comment
Share on other sites

Wait, I just got rid of the function and its button, and then rewrote it the same way and it worked. Thanks for the help with the quotes, I tried them earlier and it hadn't worked, but after retyping the button and the function the quotes did help!

Link to comment
Share on other sites

Ok, when I read the subject line I thought of something like this...

<!DOCTYPE html><html lang="en"><head><meta charset="utf-8"/><title>title</title><style></style><script>window.onerror = function(a, b, c, d){alert('Javascript Error:n'+a+'nURL: '+b+'nLine: '+c+'  Column: '+d);return true;}</script><script>'use strict';function colour(){ var r = Math.floor( Math.random() * 256 ).toString(16); var g = Math.floor( Math.random() * 256 ).toString(16); var b = Math.floor( Math.random() * 256 ).toString(16); r = (r.length==2) ? r : '0'+r; g = (g.length==2) ? g : '0'+g; b = (b.length==2) ? b : '0'+b; var c = '#' + r + g + b; document.getElementById('num').textContent = c; document.getElementsByTagName('BODY')[0].style.backgroundColor = c; setTimeout(colour, 250);}window.onload = function(){colour();}</script></head><body><h1>Hello Random World!</h1><span id="num"></span></body></html>

...but I am curious about your object-oriented approach, which is essentially the same as what you had used in some other posts, but really this is not a practical approach. Not when there is just one property and no reason to bother with creating object instances.

Link to comment
Share on other sites

Oh, well I have a few reasons. I learned what little knowledge I have about computer programming, which means very basic html, css, and javascript, on a website called codecademy. On this website you can create codebits and it provides enough sheets, one for each language that is. So I have an html sheet, a css sheet, and a javascript sheet. They are all linked, and it keeps it so organized it is really helpful. Afterwards I just create three seperate documents using notepad, link them together, and it makes it much easier to organize. Also, codecademy didn't teach me everything about the more in depth set up of a code, just the basic skeleton of each seperate language. Does that all make sense? Basically it helps with organization to use three seperate sheets, plus, I haven't learned all about how to set up everything, such as the window.onerror thing, or the meta tag. I just keep it simple and organized. Although that sometimes means creating objects that I dont need, it keeps it simple.

Link to comment
Share on other sites

Well, I seem to see a lot of beginners who get confused because they have been taught to break everything up into separate files. Yes indeed it makes perfect sense to break things up once each section won't fit on the screen due to its length, but for small demos it is more practical to keep everything as one file, as I showed above. As long as the <body> fits on one screen and the <style> fits on one screen and the <script> fits on one screen, then there is no reason to break these out into their own file. Likewise it makes little sense to instantiate objects from a constructor when the object consists of only one property. Also there are other ways to create objects other than using a constructor.

Link to comment
Share on other sites

It's very overkill to create an object color() with a single property color just to hold a string and assign it to a variable with the very same name as the value of the string.

 

That's like three levels of unnecessary abstraction. If you need a list of colors, make a single array of strings:

var colors = ["red", "green", "blue"];

As a general rule, if your object doesn't have methods (functions) in it, there's no need to make a constructor for it.

Link to comment
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
×
×
  • Create New...