More outsourced madness



  • My company hired an outsourced "enhancement" team to add some ajax stuff to a few pages. Stuff that my team could have done, but didn't have the time/bandwidth.

    Its some of the worst code  I've ever seen.

     Behold:

      if(document.getElementById('tdZipButton')!=null)
                        {
                            document.getElementById('tdZipButton').innerHTML=document.getElementById('tdZipButton').innerHTML.replace('id="zipCode_btnAjaxContinue"','id="zipCode_btnAjaxContinue" onclick="return CheckZipClick()"')
                            document.getElementById('tdZipButton').innerHTML=document.getElementById('tdZipButton').innerHTML.replace("id='zipCode_btnAjaxContinue'","id='zipCode_btnAjaxContinue' onclick='return CheckZipClick()'")                      
                            document.getElementById('tdZipButton').innerHTML=document.getElementById('tdZipButton').innerHTML.replace('ID="zipCode_btnAjaxContinue"','ID="zipCode_btnAjaxContinue" onclick="return CheckZipClick()"')
                            document.getElementById('tdZipButton').innerHTML=document.getElementById('tdZipButton').innerHTML.replace("ID='zipCode_btnAjaxContinue'","ID='zipCode_btnAjaxContinue' onclick='return CheckZipClick()'")                      
                            document.getElementById('tdZipButton').innerHTML=document.getElementById('tdZipButton').innerHTML.replace("id=zipCode_btnAjaxContinue","id='zipCode_btnAjaxContinue' onclick='return CheckZipClick()'")
                        }

     Theres about 10 WTF's right there. Imagine seeing 300-400 lines like this.



  • The Real WTF(tm) is that they could have simply written

    if(document.getElementById('tdZipButton'))



  • How about not having to use GetElementById everytime they reference the element.

     Why are they using the parent Elements innerHTML property and replace method to change attributes on a child element. What horrible break in logic would make anyone think thats a good idea.

    Also, the child element [b]DOES[/b] have a unique ID!

     



  • @Jonathan Holland said:

    My company hired an outsourced "enhancement" team to add some ajax stuff to a few pages. Stuff that my team could have done, but didn't have the time/bandwidth.

    Its some of the worst code  I've ever seen.

     Behold:

      if(document.getElementById('tdZipButton')!=null)
                        {
                            document.getElementById('tdZipButton').innerHTML=document.getElementById('tdZipButton').innerHTML.replace('id="zipCode_btnAjaxContinue"','id="zipCode_btnAjaxContinue" onclick="return CheckZipClick()"')
                            document.getElementById('tdZipButton').innerHTML=document.getElementById('tdZipButton').innerHTML.replace("id='zipCode_btnAjaxContinue'","id='zipCode_btnAjaxContinue' onclick='return CheckZipClick()'")                      
                            document.getElementById('tdZipButton').innerHTML=document.getElementById('tdZipButton').innerHTML.replace('ID="zipCode_btnAjaxContinue"','ID="zipCode_btnAjaxContinue" onclick="return CheckZipClick()"')
                            document.getElementById('tdZipButton').innerHTML=document.getElementById('tdZipButton').innerHTML.replace("ID='zipCode_btnAjaxContinue'","ID='zipCode_btnAjaxContinue' onclick='return CheckZipClick()'")                      
                            document.getElementById('tdZipButton').innerHTML=document.getElementById('tdZipButton').innerHTML.replace("id=zipCode_btnAjaxContinue","id='zipCode_btnAjaxContinue' onclick='return CheckZipClick()'")
                        }

     Theres about 10 WTF's right there. Imagine seeing 300-400 lines like this.

     

     

    What do you expect from a bunch of sand people?  Have you seen them attempt simple html?  I don't think they know what a close html tag is.  



  • No need to be racist. I've worked with some excellent indian programmers. The good ones come here. The crappy ones tend to stay there.

    Here is my fix:

     var zipCodebtnAjaxContinue = document.getElementById('zipCode_btnAjaxContinue');
                       
                        if (zipCodebtnAjaxContinue)
                        {
                            zipCodebtnAjaxContinue.onClick = CheckZipClick();
                        }



  • @Lysis said:

    What do you expect from a bunch of sand people?  Have you seen them attempt simple html?  I don't think they know what a close html tag is.

    Sand people? You do realise there are more than one places outside your little part of the world, right? It's not all sand populated with people that write bad code and hate you. I know that's all you see on the news, but somehow they have neglected to tell you that there are a few billion people out there going about their business. Hell, a lot of them are far more competent developers than you or me.

    I've studied with indian people during my Comp Sci degree and I can guarantee you they can do far more than close a HTML document



  • @Jonathan Holland said:

    Here is my fix:

     var zipCodebtnAjaxContinue = document.getElementById('zipCode_btnAjaxContinue');
                       
                        if (zipCodebtnAjaxContinue)
                        {
                            zipCodebtnAjaxContinue.onClick = CheckZipClick();
                        }

    TRWTF



  • @DOA said:

    @Lysis said:

    What do you expect from a bunch of sand people?  Have you seen them attempt simple html?  I don't think they know what a close html tag is.

    Sand people? You do realise there are more than one places outside your little part of the world, right? It's not all sand populated with people that write bad code and hate you. I know that's all you see on the news, but somehow they have neglected to tell you that there are a few billion people out there going about their business. Hell, a lot of them are far more competent developers than you or me.

    I've studied with indian people during my Comp Sci degree and I can guarantee you they can do far more than close a HTML document

     

     

    I'm sure there will be plenty of people who will tell me about the few Injuns they know who are really goood good coderz!!!!1!!!  We all know that stereotype comes from truth.  Using your scenario, you do realize that outside of those few Injuns you have interacted with that there are millions of crappy coding dirty ones out there too that fit my description of them.



  • @Lysis said:

    I'm sure there will be plenty of people who will tell me about the few Injuns they know who are really goood good coderz!!!!1!!!  We all know that stereotype comes from truth.  Using your scenario, you do realize that outside of those few Injuns you have interacted with that there are millions of crappy coding dirty ones out there too that fit my description of them.

     

     Perhaps, but I have the ability to not be a total douchbag about it.

     



  • @Jonathan Holland said:

    No need to be racist. I've worked with some excellent indian programmers. The good ones come here. The crappy ones tend to stay there.

    Here is my fix:

     var zipCodebtnAjaxContinue = document.getElementById('zipCode_btnAjaxContinue');
                       
                        if (zipCodebtnAjaxContinue)
                        {
                            zipCodebtnAjaxContinue.onClick = CheckZipClick();
                        }

    Might want to remove the parentheses after CheckZipClick there. If I'm not mistaken, your current solution assigns the result of the evaluation of the function CheckZipClick to the onClick event handler, rather than the function itself.



  • Yeah, actually I typed that into this editor here. The actual code fix I used replaced CheckZipClick with an anonymous function closure.

     ie:

     

    zipCodebtnAjaxContinue.onclick = function () 

    {

    // Do the zipcode stuff. 

    } ;



  • @Lysis said:

    I'm sure there will be plenty of people who will tell me about the few Injuns they know who are really goood good coderz!!!!1!!!  We all know that stereotype comes from truth.  Using your scenario, you do realize that outside of those few Injuns you have interacted with that there are millions of crappy coding dirty ones out there too that fit my description of them.

    And while you and the people you know might be "really goood good coderz", for every one of those there are a ton of crappy coding dirty Americans and Europeans as well. You might as well respond to every other thread on this board with. "What do you expect from a bunch of crackers?"

    Grow up



  • @SuperousOxide said:

    You might as well respond to every other thread on this board with. "What do you expect from a bunch of crackers?"
     

    That is pretty likely to happen now. Thanks for giving him the suggestion.



  • @MasterPlanSoftware said:

    @SuperousOxide said:

    You might as well respond to every other thread on this board with. "What do you expect from a bunch of crackers?"
     

    That is pretty likely to happen now. Thanks for giving him the suggestion.

     

     

    SuperiousOxidous has a great idea! 



  • @SuperousOxide said:

    for every one of those there are a ton of crappy coding dirty Americans and Europeans as well.
     

      I like to refer to those people you describe as "black people." 



  • @Lysis said:

    @MasterPlanSoftware said:

    @SuperousOxide said:

    You might as well respond to every other thread on this board with. "What do you expect from a bunch of crackers?"
     

    That is pretty likely to happen now. Thanks for giving him the suggestion.

    SuperiousOxidous has a great idea! 

     

    And the point of your existence is what, exactly?



  • @belgariontheking said:

    @Lysis said:

    @MasterPlanSoftware said:

    @SuperousOxide said:

    You might as well respond to every other thread on this board with. "What do you expect from a bunch of crackers?"
     

    That is pretty likely to happen now. Thanks for giving him the suggestion.

    SuperiousOxidous has a great idea! 

     

    And the point of your existence is what, exactly?

     

     

    The same reason you post here? 



  •  TunnelRat? Is that you?



  • @Jonathan Holland said:

    replace method to change attributes on a child element.
     

    IE's particularly borked and refuses to let you change or set attributes on elements, usually at random. Doing x.setAttribute('attribute', 'value') works for some, but fails for others, while doing "x[attribute] = 'value'" works/doesn't work for yet another set. Sometimes it's just easier to rebuild the tag using innerHTML, even if it isn't "best practices". 



  •  setAttribute('click',somefunction);

     or 

    element.onclick = somefunction;

     Works just fine in IE.

    There is no excuse for this. 



  • @Lysis said:

    What do you expect from a bunch of sand people?  Have you seen them attempt simple html?  I don't think they know what a close html tag is.  

     

    <IgnoreBlatantRasicm> 

    Sand people?  I didn't know people outsourced to Tusken Raiders nowadays!

    </IgnoreBlatantRacism> 



  • You are living up to your name wise Jedi.

     Personally, when I think of India, I think of humidity, monsoon rains, and jungles...I don't think of deserts.

     Lysis, perhaps both a Political Correctness class and a Geography class should be required?



  • @ObiWayneKenobi said:

    Sand people?  I didn't know people outsourced to Tusken Raiders nowadays!

    But just picture an Indian standing up in his cubicle, holding his keyboard above his head and doing that grunt.



  • @Jonathan Holland said:

    You are living up to your name wise Jedi.

     Personally, when I think of India, I think of humidity, monsoon rains, and jungles...I don't think of deserts.

     Lysis, perhaps both a Political Correctness class and a Geography class should be required?

     

     I think a severe thrashing with the clue stick would be more to the point. That, or being mauled by a bear. Him/her/it and all his/her/its "grate coader" friends.



  • Hasn't anyone heard of Prototype? It makes JavaScript 50 easy...

    var button = $('tdZipButton');

    button && button.onclick = CheckZipClick;



  • @Cap'n Steve said:

    @ObiWayneKenobi said:
    Sand people?  I didn't know people outsourced to Tusken Raiders nowadays!

    But just picture an Indian standing up in his cubicle, holding his keyboard above his head and doing that grunt.

     

    Okay, I'll admit that would be funny.   But that'd be funny no matter the person's ethnicity.

     I say we lock Lysis in a room with SpectateSwamp and make him/her/it use SSDS for the rest of his/her/its life.  For everything.  If we're lucky, he/she/it will commit suicide, and SpectateSwamp will, for once, be praised instead of ridiculed.



  • @MarcB said:

    IE's particularly borked and refuses to let you change or set attributes on elements, usually at random. Doing x.setAttribute('attribute', 'value') works for some, but fails for others, while doing "x[attribute] = 'value'" works/doesn't work for yet another set. Sometimes it's just easier to rebuild the tag using innerHTML, even if it isn't "best practices". 

     

     

    And sometimes IE can break innerHTML in some very creative ways.  For a non-standard that MS came up with, its fun.

     

    <table> is broken rather badly, though <tbody>  works.   IIRC,  <select> doesn't work well either.

    <title> has some weird issues.

    A handful of tags have problems with event handlers not working...

     



  • @ActionMan said:

    Hasn't anyone heard of Prototype? It makes JavaScript 50 easy...
     

    Yup, heard of it.

    I will admit I did steal the $() function from an early version of prototype, but then as I got better I realized  I needed it less and less.  (Not gone, don't get me wrong, but document.forms.myform.myelement often works better.

    Prototype has a number of issues one should be aware of before deploying on a business site.  If you aren't sure you can fix it WHEN it breaks, don't use it.  (It does a fair amount of browser sniffing and doesn't play well with "non-standard" user-agents including cellphones and PDAs.  Search comp.lang.javascript for more than you'll ever want to hear on the subject.) 



  • @r3jjs said:

    And sometimes IE can break innerHTML in some very creative ways.  For a non-standard that MS came up with, its fun.

     

    <table> is broken rather badly, though <tbody>  works.   IIRC,  <select> doesn't work well either.

    It's not just IE. Using innerHTML on a table was broken in Opera (not sure if it still is) and Firefox 2.x has some weird <select> bugs.



  • @ActionMan said:

    var button = $('tdZipButton');

    button && button.onclick = CheckZipClick;

    If you already use perl-style shortcut abuse, you might as well remove the variable too:

    ( $('tdZipButton') || {} ).onclick = CheckZipClick;


  • Considered Harmful

    @PSWorx said:

    ( $('tdZipButton') || {} ).onclick = CheckZipClick;

    Wouldn't that leak a little bit of memory by creating a new unused object { 'onclick': CheckZipClick }?

    Not that memory leaks are a huge issue in Javascript. The browsers do enough leaking on their own.

    Is there some kind of garbage collector in JavaScript?



  • @joe.edwards@imaginuity.com said:

    @PSWorx said:

    ( $('tdZipButton') || {} ).onclick = CheckZipClick;

    Wouldn't that leak a little bit of memory by creating a new unused object { 'onclick': CheckZipClick }?

    Not that memory leaks are a huge issue in Javascript. The browsers do enough leaking on their own.

    Is there some kind of garbage collector in JavaScript?

     

    There is, which is why this snippet wouldn't really cause any harm in the long run.

    The collector isn't run very often in most implementations, from what I know, so code like this could cause an unnecessary large memory footprint if run, for example, inside a loop. 



  • (*Kicks the forum*)

    With which I wanted to say, I'm aware that this snippet is inefficient in memory usage, but it's far superior in source-code-length-efficiency ;) 



  • @ActionMan said:

    Hasn't anyone heard of Prototype? It makes JavaScript 50 easy...

    var button = $('tdZipButton');

    button && button.onclick = CheckZipClick;

     

    I prefer jQuery.

    $('#tdZipButton').click(CheckZipClick)

    Although that's probably already been superseded by <insert name of JavaScript library du jour>



  • @Jonathan Holland said:

    You are living up to your name wise Jedi.

     Personally, when I think of India, I think of humidity, monsoon rains, and jungles...I don't think of deserts.

     Lysis, perhaps both a Political Correctness class and a Geography class should be required?

     

     

    Really? Whenever I think of India, I think of this guy:

     http://believe-or-not.blogspot.com/2007/11/man-in-india-marries-dog.html

    Man marries his dog


Log in to reply