Page 1 of 2 12 LastLast
Results 1 to 10 of 13

Thread: My JavaScript is broken.

  1. #1
    Join Date
    Feb 2007
    Location
    🌎
    Posts
    528
    Thanks
    10
    Thanked 10 Times in 10 Posts
    Blog Entries
    2

    Default My JavaScript is broken.

    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>
    ....(o_ Penguins
    .---/(o_- techno_racing
    +(---//\-' in
    .+(_)--(_)' The McMurdo 500

  2. #2
    Join Date
    Jun 2005
    Location
    英国
    Posts
    11,876
    Thanks
    1
    Thanked 180 Times in 172 Posts
    Blog Entries
    2

    Default

    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.
    Twey | I understand English | 日本語が分かります | mi jimpe fi le jbobau | mi esperanton komprenas | je comprends franšais | entiendo espa˝ol | t˘i Ýt hiểu tiếng Việt | ich verstehe ein bisschen Deutsch | beware XHTML | common coding mistakes | tutorials | various stuff | argh PHP!

  3. #3
    Join Date
    Jul 2008
    Posts
    65
    Thanks
    42
    Thanked 0 Times in 0 Posts

    Default

    You forgot a semicolon in your onclick, and a bracket in one of your ifs... I don't even get the rest :P
    Last edited by hosdank; 08-22-2008 at 06:33 PM.

  4. #4
    Join Date
    Jan 2008
    Posts
    4,168
    Thanks
    28
    Thanked 628 Times in 624 Posts
    Blog Entries
    1

    Default

    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("");
    }
    Last edited by Nile; 08-22-2008 at 07:49 PM.
    Jeremy | jfein.net

  5. #5
    Join Date
    Jul 2006
    Location
    Canada
    Posts
    2,581
    Thanks
    13
    Thanked 28 Times in 28 Posts

    Default

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

  6. #6
    Join Date
    Jan 2008
    Posts
    4,168
    Thanks
    28
    Thanked 628 Times in 624 Posts
    Blog Entries
    1

    Default

    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.
    Jeremy | jfein.net

  7. #7
    Join Date
    Jul 2006
    Location
    Canada
    Posts
    2,581
    Thanks
    13
    Thanked 28 Times in 28 Posts

    Default

    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>
    - Mike

  8. #8
    Join Date
    Jul 2006
    Location
    Canada
    Posts
    2,581
    Thanks
    13
    Thanked 28 Times in 28 Posts

    Default

    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>
    - Mike

  9. #9
    Join Date
    Jun 2005
    Location
    英国
    Posts
    11,876
    Thanks
    1
    Thanked 180 Times in 172 Posts
    Blog Entries
    2

    Default

    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.
    Twey | I understand English | 日本語が分かります | mi jimpe fi le jbobau | mi esperanton komprenas | je comprends franšais | entiendo espa˝ol | t˘i Ýt hiểu tiếng Việt | ich verstehe ein bisschen Deutsch | beware XHTML | common coding mistakes | tutorials | various stuff | argh PHP!

  10. #10
    Join Date
    Jan 2008
    Posts
    4,168
    Thanks
    28
    Thanked 628 Times in 624 Posts
    Blog Entries
    1

    Default

    Twey, like I said?
    (Laughs at Mburt, saying I told you!)
    Jeremy | jfein.net

Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •