My JavaScript is broken.

**techno_race** · 08-22-2008, 12:44 AM

As usual.
The idea is to take a user-submitted hex color code, split it into its R, G, and B parts, and then display them. What's wrong with this picture?

HTML Code:

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd">
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<title>Color Splitter</title>
<script type="text/javascript">
function splitColors()
{
	var input = document.getElementById('input').value;
	var array = input.split("");
	var result = array;
	if (array[0] == "#") {
		result = array.splice(0,1);
	}
	var ah = array.length;
	if (ah == 3) {
		var v1 = result[0];
		var v2 = result[1];
		var v3 = result[2];
		result = [v1,v1,v2,v2,v3,v3];
	}
	if (ah == 0) {
		result = [0,0,0,0,0,0];
	if (ah == 1) {
		var old = result[0];
		result = [old,old,old,old,old,old];
	}
	if (ah == 2) {
		var v1 = result[0];
		var v2 = result[1];
		result = [v1,v2,v1,v2,v1,v2];
	}
	if (ah == 4) {
		var v1 = result[0];
		var v2 = result[1];
		var v3 = result[2];
		var v4 = result[3];
		result = [v1,v2,v3,v4,v1,v2];
	}
	if (ah == 5) {
		var v1 = result[0];
		var v2 = result[1];
		var v3 = result[2];
		var v4 = result[3];
		var v5 = result[4];
		result = [v1,v2,v3,v4,v5,v1];
	}
	if (ah == 7) {
		var old = result;
		result = old.splice(6,1); //No pun intended
	}
	if (result[0] != "0" || result[0] != "1" || result[0] != "2" || result[0] != "3" || result[0] != "4" || result[0] != "5" || result[0] != "6" || result[0] != "7" || result[0] != "8" || result[0] != "9" || result[0] != "A" || result[0] != "B" || result[0] != "C" || result[0] != "D" || result[0] != "E" || result[0] != "F" || result[0] != "a" || result[0] != "b" || result[0] != "c" || result[0] != "d" || result[0] != "e" || result[0] != "f") {
		result[0] = 'F';
	}
	if (result[1] != "0" || result[1] != "1" || result[1] != "2" || result[1] != "3" || result[1] != "4" || result[1] != "5" || result[1] != "6" || result[1] != "7" || result[1] != "8" || result[1] != "9" || result[1] != "A" || result[1] != "B" || result[1] != "C" || result[1] != "D" || result[1] != "E" || result[1] != "F" || result[1] != "a" || result[1] != "b" || result[1] != "c" || result[1] != "d" || result[1] != "e" || result[1] != "f") {
		result[1] = 'F';
	}
	if (result[2] != "0" || result[2] != "1" || result[2] != "2" || result[2] != "3" || result[2] != "4" || result[2] != "5" || result[2] != "6" || result[2] != "7" || result[2] != "8" || result[2] != "9" || result[2] != "A" || result[2] != "B" || result[2] != "C" || result[2] != "D" || result[2] != "E" || result[2] != "F" || result[2] != "a" || result[2] != "b" || result[2] != "c" || result[2] != "d" || result[2] != "e" || result[2] != "f") {
		result[2] = 'F';
	}
	if (result[3] != "0" || result[3] != "1" || result[3] != "2" || result[3] != "3" || result[3] != "4" || result[3] != "5" || result[3] != "6" || result[3] != "7" || result[3] != "8" || result[3] != "9" || result[3] != "A" || result[3] != "B" || result[3] != "C" || result[3] != "D" || result[3] != "E" || result[3] != "F" || result[3] != "a" || result[3] != "b" || result[3] != "c" || result[3] != "d" || result[3] != "e" || result[3] != "f") {
		result[3] = 'F';
	}
	if (result[4] != "0" || result[4] != "1" || result[4] != "2" || result[4] != "3" || result[4] != "4" || result[4] != "5" || result[4] != "6" || result[4] != "7" || result[4] != "8" || result[4] != "9" || result[4] != "A" || result[4] != "B" || result[4] != "C" || result[4] != "D" || result[4] != "E" || result[4] != "F" || result[4] != "a" || result[4] != "b" || result[4] != "c" || result[4] != "d" || result[4] != "e" || result[4] != "f") {
		result[4] = 'F';
	}
	if (result[5] != "0" || result[5] != "1" || result[5] != "2" || result[5] != "3" || result[5] != "4" || result[5] != "5" || result[5] != "6" || result[5] != "7" || result[5] != "8" || result[5] != "9" || result[5] != "A" || result[5] != "B" || result[5] != "C" || result[5] != "D" || result[5] != "E" || result[5] != "F" || result[5] != "a" || result[5] != "b" || result[5] != "c" || result[5] != "d" || result[5] != "e" || result[5] != "f") {
		result[5] = 'F';
	}
	var red = result[0] + "" + result[1];
	var green = result[2] + "" + result[3];
	var blue = result[4] + "" + result[5];
	var full = red + "" + green + "" + blue;
	var newHTML = '<span style="background-color: #' + full + ';">' + full + '</span><br>is made of<br><span style="background-color: #' + red + '0000;">' + red + '0000</span><br><span style="background-color: #00' + green + '00;">00' + green + '00</span><br><span style="background-color: #0000' + blue + ';">0000' + blue + '</span>';
	document.getElementById('output').innerHTML = newHTML;
}
</script>
</head>

<body>
<input name="input" type="text" id="input" size="12" maxlength="7">
<input type="button" name="button" id="button" value="Split" onClick="splitColors()">
<div id="output">Your splitted color will be here.</div>
</body>
</html>

**Twey** · 08-22-2008, 06:06 PM

There's too much useless code in there for me to feel like reading it, but you may get some hints from this post.

Stay away from innerHTML — it's non-standard and will make your life harder in the long run.

**hosdank** · 08-22-2008, 06:27 PM

You forgot a semicolon in your onclick, and a bracket in one of your ifs... I don't even get the rest :P

**Nile** · 08-22-2008, 07:43 PM

It looks like this is a lot shorted then yours:
http://www.linuxtopia.org/online_boo...q/hextorgb.htm

If there's anything else your looking for other then that. You can add those onto that script.
Also, one of the reasons your javascript was broken was because you forgot to end your function.

Again, like Twey said, try to make your code shorter.
Thats important when worrying about file size. Its also a bad habbit.
Also, when makign code, don't use the [TAB] button, here's what your code looks like(example):

Code:

function(e){
	e.split("");
}

Here's a shorter way:

Code:

function(e){
     e.split("");
}

**mburt** · 08-22-2008, 09:48 PM

I believe Twey was referring to the overuse of if commands and whatnot...

**Nile** · 08-22-2008, 09:57 PM

Mburt, it also could be a lot shorter with the use of short if statements, for loops, and having lines such as:

Code:

	var red = result[0] + "" + result[1];
	var green = result[2] + "" + result[3];
	var blue = result[4] + "" + result[5];

Put it a proper format such as:

Code:

     var red = result[0] + result[1];
     var green = result[2] + result[3];
     var blue = result[4] + result[5];

And even turning those variables into an array would be a bit more orgainized.

**mburt** · 08-22-2008, 10:01 PM

The point of this script is to change hex into rgb, then display right?

This script is shorter:

Code:

<script type="text/javascript">
_ = function(e) { return document.getElementById(e); };
</script>
<div id="object">&nbsp;</div>
<input type="text" id="foo">
<input type="button" value="Test" onclick="_('object').style.background = 'rgb('+rgb(_('foo').value)+')'">
<script type="text/javascript">
var rgb = function(hex) {
  hex = hex.replace(/\#/i, "")
  for (var i = 2, str = "";i <= 6;i=i+2) {
    str += parseInt(hex.substring(i-2, i), 16)+", ";
  };
  return str.substring(0, str.length-2);
};
</script>

**mburt** · 08-22-2008, 10:35 PM

Actually... After looking at it, I don't know why I always use the "str += ", when I could simply use an array, then join it:

Code:

<script type="text/javascript">
_ = function(e) { return document.getElementById(e); };
</script>
<div id="object">&nbsp;</div>
<input type="text" id="foo">
<input type="button" value="Test" onclick="_('object').style.background = 'rgb('+rgb(_('foo').value)+')'">
<script type="text/javascript">
var rgb = function(hex) {
  for (var i = 2, out = [];i <= 6;i=i+2) {
    out[(i/2)-1] = parseInt((hex.replace(/\#/i, "")).substring(i-2, i), 16);
  };
  return out.join(", ");
};
</script>

**Twey** · 08-22-2008, 10:38 PM

I was referring to the repetitiveness of the code. Computers exist so that you don't have to perform the same repetitions over and over again manually. If you find yourself doing so, you're doing it wrong and need to abstract more — try a loop, for example.

mburt, your code has errors.

I refer you once again to my Colour object in the post to which I linked above, which contains code for converting to/from various string formats.

**Nile** · 08-22-2008, 10:40 PM

Twey, like I said?
(Laughs at Mburt, saying I told you!)

Thread: My JavaScript is broken.

Thread Tools

Search Thread

My JavaScript is broken.

Bookmarks

Bookmarks

Posting Permissions