• CaptainObvious (unregistered) in reply to attemp1

    Its really easy:

    if (elementItem == "header1:SearchBox") { __doPostBack('header1:goSearch',''); } else if (Regex.isMatch("Text[0-9]+",elementItem)) { window.event.returnValue=false; window.event.cancel = true; document.forms[0].elements[n+1].focus(); } Not sure about regex in js so i used what i can vaguely remember from .net syntax.

    This will match up to Text99999999999999999999999999999999999999999999999999999999999999999999999999 too, as a bonus.

  • Alessandro (unregistered) in reply to attemp1

    Maybe: switch (elementItem) { case "header1:SearchBox" : { __doPostBack('header1:goSearch',''); break; } case "onetoignore": break default: { window.event.returnValue=false; window.event.cancel = true; document.forms[0].elements[n+1].focus(); break; } }

  • ZiggyFish (unregistered)

    It's simple (or supposedly) form validation.

    function submitTrap3(){ var i; var elementItem;

    __doPostBack('header1:goSearch',''); for (n=0;n<document.forms[0].elements.length;n++){ elementItem = document.forms[0].elements[n].name;

      if ((elementItem.subString(0,4) == "text") && (document.forms[0].elements[n].value != "") && (elementItem == trim(document.forms[0].txtControlName.value)))
     return false;

    } }

  • Foo (unregistered) in reply to attemp1
    attemp1:

    I just can't see anything else that would make it smaller, expandable and easier to manage...

    How about?

    var n = parseInt(elementItem.substr(5)); var s = elementItem.substr(0, 4); if (s == "Text" && n >= 1 && n <= 50) { ... }

  • James (unregistered)

    So many people suggesting this solution.

    for (n=0;n<document.forms[0].elements.length;n++) { . .

    default: { window.event.returnValue=false; window.event.cancel = true; document.forms[0].elements[n+1].focus(); break; }

    Nice keep up the good work.

  • ZiggyFish (unregistered) in reply to Foo

    Tell me what happens when elementitem == 'Text1a'?

  • ZiggyFish (unregistered) in reply to Foo
    Foo:
    attemp1:

    I just can't see anything else that would make it smaller, expandable and easier to manage...

    How about?

    var n = parseInt(elementItem.substr(5)); var s = elementItem.substr(0, 4); if (s == "Text" && n >= 1 && n <= 50) { ... }

    Tell me what happens when elementitem == 'Text1a'?

  • Shaun B (unregistered)

    Unless there's a default statement, why have the switch at all? Why not just have the block of code?

  • Adam Jorgensen (unregistered) in reply to attemp1

    Failure.

    Please apply a regular expression and retry :-)

  • David (unregistered)

    Hasn't anyone thought of this:

    //Pseudo code

    if (elementname=="header1:SearchBox"){ ... }elseif (elementname like 'Text*'){ window.event.returnValue=false; window.event.cancel = true; document.forms[0].elements[n+1].focus(); }

  • Phil (unregistered) in reply to halcyon1234

    In vb could be reduced to…

    dim index as integer

    index = val(mid(elementitem,4,elementitem.length))

    If index > 0 and index < 50 then window.event.returnValue=false window.event.cancel = true document.forms[0].elements[n+1].focus() break end if

  • (cs)

    I can't believe no-one's even bothered looking at this yet:

    Article:
    elementItem = document.forms[0].elements[n].name;
if ((document.forms[0].elements[n].value != "") &&
(elementItem == trim(document.forms[0].txtControlName.value)))
    So... the element name is being compared with a textbox in the form to check that boxes value is the same as the element name before it goes through the lengthy switch? WHAT?!?

    For my tuppence, assuming the odd spacing was an anonymisation artifact I'd go for this being autogen'd - given the __postBack and the TextN naming probably by a recent version of FrontPage, if not Visual Studio... Gods how I hate Microsoft technology...

  • guest (unregistered) in reply to halcyon1234

    FAIL.

  • Fast Eddie (unregistered)

    IF elementItem MATCH "Text'2N'" THEN IF elementItem[5, 2] > 0 AND elementItem[5, 2] < 51 THEN * DO STUFF EXIT END END

    Oh wait...Your language doesn't HAVE pattern matching.

    nm

  • rgz (unregistered)

    Ok so what's the thing /s?he/ is doing with txtControlName?

    function submitTrap3(){ var elements = document.forms[0].elements if (window.event.keyCode == 13){ for(var e in elements){ var hasValue = elements[n].value != "" var sayWhat = elements[e].name == trim(document.forms[0].txtControlName.value) var matches = elements[e].name.match(/Text[0-9]+/) if (hasValue && sayWhat && matches){ window.event.returnValue = false window.event.cancel = true elements[e + 1].focus() } } // Special cased as I refuse to test 53 times for it var header = document.getElementsByName("header1:SearchBox")[0] var hasValue = header.value != "" var sayWhat = header.name == trim(document.forms[0].txtControlName.value) if (hasValue && sayWhat){ __doPostBack('header1:goSearch','') } } }

  • BloodGain (unregistered)

    Oh, I see! He forgot to put in the default case!</sarcasm>

    No, but seriously -- there should still be a default case, even if it's just commented out:

    //default:
//no action otherwise

    This makes it clearer to a maintainer that you didn't intend to handle cases that aren't given, or that they aren't expected. If they simply aren't expected, some error checking might be in order.

  • God (unregistered) in reply to attemp1

    Try parsing the int at the end, I havent used vb in a while but in python...

    if elementItem == header1:SearchBox: __doPostBack('header1:goSearch',''); elif elementItem.startswith("Text") and int(elementItem[4:]) <= 50: window.event.returnValue=false; window.event.cancel = true; document.forms[0].elements[n+1].focus();

  • Brad Wood (unregistered) in reply to attemp1

    There's always more you can do. :)

    Use some Regex to pull the number out of "Text00", treat it as a numberic and check if (the_number > 0 && < 51)

  • Anonymous (unregistered)

    That's a lot shorter...

    if (elementItem == "header1:SearchBox") {
	__doPostBack('header1:goSearch','');
} else if (/^Text([1-9]|[1-4][0-9]|50)$/.match(elementItem)) {
	window.event.returnValue=false;
	window.event.cancel = true;
	document.forms[0].elements[n+1].focus();
}
  • Giambo (unregistered)

    Regexp ! Use the holy regexps :) !

  • Spiked (unregistered)

    I hope the 2 of you are joking....

    function submitTrap3() {
 if (window.event.keyCode == 13) {
  for (var n = 0; n < document.forms[0].elements.length; n++) {
   var elementItem = document.forms[0].elements[n].name;
   if ((document.forms[0].elements[n].value != "") && (elementItem == trim(document.forms[0].txtControlName.value))) {
    if ( elementItem == "header1:SearchBox" ) {
     __doPostBack('header1:goSearch','');
    } else if ( elementItem.substring(0,4) == "Test" ) {
      window.event.returnValue=false;
      window.event.cancel = true;
      document.forms[0].elements[n+1].focus();
    }
   }
  }
 }
}
  • Pedro (unregistered) in reply to attemp1
    attemp1:
    how about this?

    (snip)

    I just can't see anything else that would make it smaller, expandable and easier to manage...

    Well, testing for "Text{0-9}" would conflate all of those into just one case.

  • Nile (unregistered) in reply to JoelKatz

    Defend it?

    Firstly: convenience... It's a stretch, but I can imagine seeing that if I'd automatically-generated it at the very start of implementing a branched structure. Like, it's temporary, and I'll get 'round to filling it in.

    ...We will draw a kindly veil over the likelihood that this is production code, and that the author never thought of Case ELSE.

    Secondly: efficiency... In a well-designed environment, you could put the most-often-chosen option at the top: CASE structures do not read each and every statement, they branch out of the structure on reading the first 'Case=TRUE' condition. If the sequence runs down the list in descending order of popularity, this might be a good performance decision.

    ...But I don't buy that, as it doesn't look like that's the design; and I'm not so sure that there are any well-designed compilers that would give a noticable performance gain after loading the CASE structure. And it's moot: this looks like it's loading user interface components: it isn't a performance-critical 'bottleneck' in the application.

    That's the limit of 'defending' it. Am I missing something?

    Like mockery?

    No, I will put in the obvious point: separately-elaborated Case structures of more than eight branches suggest a failure of design.

    Firstly, I'd look at the underlying logic again: could it be rearranged hierarchically, adding successive branching steps to an individual path? Surely the choices can be grouped! Yes, there's more steps between input and output, but better design will reduce the overhead of loading and reading such a large Case structure.

    Secondly, consider variable substitution and automation instead of all this elaboration. That is to say, each and every case has been 'elaborated' - described separately and in full - when the outputs can be expressed as a simple function of the input (or 'Branching') variable.

    Let's simplify, and say that each and every one of those cases did something genuinely different:

    CASE='Choice1'; 'Load Form 1' CASE='Choice2'; 'Load form 2' CASE='Choice3'; 'Load Form 3'

    (Apologies to any pseudocode purists out there, but I want this to be legible to VB and SQL developers at beginner level)

    Clearly there's a case for for enumerating the inputs and outputs.

    So if the most popular input is, say, 'Choice1' - or 'Extra Fries, no onion', we might find some way of mapping that as the integer 1 in some kind of lookup structure. Or make it's ordinal position 1 in the menu that generated that response; and, by some happy coincidence, we might discover that the action we want to perform is loading the form that happens to be at ordinal 1 in the forms collection...

    Or we could have the source menu return the actual form names. Either way - names or ordinals - this is what replaces the entire Case structure:

    FormsCollection(MyMenu.LastReponse).Load

    And that's about it. CASE statements are a good way of making the branching logic 'open up' explicitly in your code: this is usually better than hiding the logic in mapping structures or matched enumerations.

    And, sometimes, the 'input' set of case control conditions is so complex and heterogeneous that only a CASE statement will do; likewise the 'ouput' set of actions that you want to perform. But outer code sample shows no such complexity!

    And if it did, I'd question the analysis that led to such a design: I've seen it on occasion, but it's actually quite rare to see such complex choices in the real world. Even multi-state vehicle insurance business applications permit hierarchical 'decision tree' structures!

    Yeah. I know. I'm stating the obvious, and doing it in baby-talk. But that's what I do for a living: I'm a VBA programmer doing spreadsheet work for options traders.

    Seriously, the only defence of that code sample is that it satisfies the essential test of clarity: at least I can see what it's doing, and there are worse things than seeing that it's wrong.

  • bOO! (unregistered) in reply to attemp1

    why not just replace all that 'case "Text##"' by a simple default ??

    ok he maybe learning to count until 50.... or trying to know how much cases js (and he) can handle...

  • Jared (unregistered)

    If value = "header1:SearchBox" then //do one thing elseif value[1:4] = "Text" and value[4:value.length-4].Integer >= 1 and value[4:value.length-4].Integer <= 50 then //do other thing endif;

    Thats how I might do it in Jade.

  • Wyrd (unregistered)

    Ok, this may not actually be working code (and it's pseudocode anyway, but I just want to get the basic idea across here, and although I'm not that familiar with javascript, surely it must be possible to do something like this: switch (elementItem) { case "header1:SearchBox" : { __doPostBack('header1:goSearch',''); break; } case else { /* now that we got here, we should still do some sort of check to make sure we don't have a weird value for elementItem that we weren't expecting */

    if the_first_for_characters_of(elementItem) = "Text" and numeric_of(all_the_rest_of_the_characters_of(elementItem)) >= 1 and numeric_of(all_the_rest_of_the_characters_of(elementItem)) <= 50 
   {
                    window.event.returnValue=false;
                    window.event.cancel = true;
                document.forms[0].elements[n+1].focus();
   }

    else { /* error debugging stuff goes here if appropriate */ } break; }

  • Chris (unregistered) in reply to attemp1

    I'm bad with javascript syntax but this 'should' do (minus typos and syntax errors. I'm a php guy)

    var i; for(i=1; i<=50; i++){ $test="Test"+i; if(elementItem==$test){ window.event.returnValue=false; window.event.cancel = true; document.forms[0].elements[n+1].focus(); return; //Assuming this whole thing is a function } }

  • mg (unregistered) in reply to attemp1

    how about

    if (regexpMatch(elementItem, '^Text\d\d?$'))

  • (cs)

    Well, it's not php. I don't see what could go wrong.

  • Ian (unregistered) in reply to attemp1

    Not sure what language that is, but if it does not suck there should be a way to use regex:

    Text([1-9]|[1-4][0-9]|50)

  • IMB (unregistered) 
    function submitTrap3()
{
    var n = 0;
    var f =0;
    var elementItem = "";
    if (window.event.keyCode == 13)
    {
        for (n=0;n<document.forms[0].elements.length;n++)
        {
            elementItem = document.forms[0].elements[n].name;
            if ((document.forms[0].elements[n].value != "") &&
                (elementItem == trim(document.forms[0].txtControlName.value)))
            {
                if ( (elementItem == "header1:SearchBox")
                {

                	__doPostBack('header1:goSearch','');
                        break;
		}
		else if ((elementItem.Length< 4) && (elementItem.substr(0,4) == "Text") && (elementItem.substr(0, elementItem.Length-4))
		{
			window.event.returnValue=false;
                        window.event.cancel = true;
                        document.forms[0].elements[n+1].focus();
                }
		else
		{
			/*something should go here? otherwise this
			 would have just been an if =="header1:SearchBox"
			 else do the Text stuff*/
		}
	      }
	}
      }
}
</pre>
  • (cs)

    On the plus side, they at least know about the switch statement. I've seen many a chained if-else and wondered "do you have something against switch/case?"

    Now someone just needs to introduce them to "default".

  • Just me (unregistered) in reply to attemp1

    Yeah! I'd do it with asm and build an entirely new compiler & linker for the job!

  • Master (unregistered)

    here we go:

    function submitTrap3()
{
    var n = 0;
    var f =0;
    var elementItem = "";
    if (window.event.keyCode == 13)
    {
        for (n=0;n<document.forms[0].elements.length;n++)
        {
            elementItem = document.forms[0].elements[n].name;
            if ((document.forms[0].elements[n].value != "") &&
                (elementItem == trim(document.forms[0].txtControlName.value)))
            {
                if (elementItem == "header1:SearchBox" ){
                        __doPostBack('header1:goSearch','');
                        break;
                 }
                 else{
                   window.event.returnValue=false;
                   window.event.cancel = true;
                   document.forms[0].elements[n+1].focus();                       
                 }        
                
            }
        }
    }
}
</pre>
  • marchaos (unregistered) in reply to attemp1

    Somehow, I don't think the switch is needed.

    if (elementItem == "header1:SearchBox") { __doPostBack('header1:goSearch',''); break; } else if (/Text(\d+)/.test() && elementItem.match(/Text(\d+)/)[0] > 1 && elementItem.match(/Text(\d+)/)[0] <= 50) { window.event.returnValue=false; window.event.cancel = true; document.forms[0].elements[n+1].focus(); }

  • HBP (unregistered) in reply to attemp1 
    if (elementItem == "header1:SearchBox") {
  __doPostBack('header1:goSearch','');
} else {
  window.event.returnValue=false;
  window.event.cancel = true;
  document.forms[0].elements[n+1].focus();
}
  • Name (unregistered)

    Just check if it starts with "Text"

  • Brandon (unregistered)

    am I the only one thinking of stripping the text and then just if( i >= 1 && i <= 50 ) { ... }

  • Zach (unregistered) in reply to attemp1

    better yet... since it looks like it's javascript.

    if elementItem == "header1:SearchBox" { __doPostBack('header1:goSearch',''); } else if(elementItem.StartsWith("Text")) { window.event.returnValue=false; window.event.cancel = true; document.forms[0].elements[n+1].focus(); }

    True, it allows for anything that starts with Text, but doesn't that appear to be the point?

  • Just a Regular Guy (unregistered) in reply to attemp1

    if (elementItem.matches(/^Text([1-5]0|[1-4]?[1-9])$/)) { window.event.returnValue=false; window.event.cancel = true; document.forms[0].elements[n+1].focus(); break; } else if (elementItem.compare("header1:SearchBox") == 0) { __doPostBack('header1:goSearch',''); break; }

  • Just a Regular Guy (unregistered) in reply to Just a Regular Guy

    No break;(s)

    And that boys and girls is why you should never cut and paste code

  • Joe (unregistered) in reply to attemp1

    switch (elementItem) { case "header1:SearchBox" : { __doPostBack('header1:goSearch',''); break; } default: { window.event.returnValue=false; window.event.cancel = true; document.forms[0].elements[n+1].focus(); break; } }

  • sebas (unregistered) in reply to cthulhu
    cthulhu:
    JoelKatz:
    I DEFY ANYONE TO DEFEND THIS CODE!

    Well it works. What's the problem?

    It builds, let's ship it!
  • Rich (unregistered)

    or something like this would work to:

                    if (elementItem == "header1:SearchBox")
                {
                     __doPostBack('header1:goSearch','');
                }
                else // Because this is the only other possible situation
                {
                    window.event.returnValue=false;
                    window.event.cancel = true;
                    document.forms[0].elements[n+1].focus();
                }
  • theY4Kman (unregistered) in reply to attemp1

    I can do you one better.

    function submitTrap3()
{
    var n = 0;
    var f =0;
    var elementItem = "";
    if (window.event.keyCode == 13)
    {
        for (n=0; n<document.forms[0].elements.length; n++)
        {
            elementItem = document.forms[0].elements[n].name;
            if ((document.forms[0].elements[n].value != "") &&
                (elementItem == trim(document.forms[0].txtControlName.value)))
            {
                if(elementItem = "header1:SearchBox")
                {
                        __doPostBack('header1:goSearch','');
                }
                else if(1 <= parseInt(elementItem.substring(4)) <= 50)
                {
                    window.event.returnValue = false;
                    window.event.cancel = true;
                    document.forms[0].elements[n+1].focus();
                }
            }
        }
    }
}
</pre>
  • Hullu Piipaa (unregistered) in reply to attemp1

    How about:

    if(elementName == "header1:SearchBox") { __doPostBack('header1:goSearch',''); break; // Or return, forgot the original code

    }

    var startIX = 1; var endIX = 50;

    for(var i = startIX; i <= endIX; i++) { var strValue = "Text" + i.toString(); if(elementName == strValue) { window.event.returnValue=false; window.event.cancel = true; document.forms[0].elements[n+1].focus(); break; // or return, if you dont need the switch } }

  • Hullu Piipaa (unregistered) in reply to Hullu Piipaa

    Owned by the browser not showin any comments (but one).

    Will... shut... up... now.

  • Akaji (unregistered)

    if (elementItem == "header1:SearchBox") { //do stuff... } else if (elementItem.match(/Text([1-9]|[1-5][0-9])/)) { //do other stuff... }

    These other comments scare me...

  • Guruuswa (unregistered) in reply to halcyon1234

    my god, is that actual code?

  • Marcelo (unregistered) in reply to attemp1

    if (elementItem.match(/^Text\d{1,2}/)) { window.event.returnValue = false; window.event.cancel = true; document.forms[0].elements[n+1].focus(); }

Leave a comment on “If I've Said It Once, I've Said It Fifty Times”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article