Jump to content
Sign in to follow this  
brooke_theperson

Randomly Changing Color

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?

Share this post


Link to post
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>

Share this post


Link to post
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.

Share this post


Link to post
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;}

Share this post


Link to post
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.

Share this post


Link to post
Share on other sites

Check your browser's console for Javascript errors. In most browsers you can see the console by pressing F12.

Share this post


Link to post
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!

Share this post


Link to post
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.

Share this post


Link to post
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.

Share this post


Link to post
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.

Share this post


Link to post
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.

Share this post


Link to post
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

Loading...
Sign in to follow this  

×
×
  • Create New...