2
\$\begingroup\$

I have written the below code to search for a string in a web page. This is working fine but I need suggestions on improving this code to start using this. This is written to work under IE.

$(document).ready(function() 
{
var NotFounds = [];
function countString(s) {
  var re = new RegExp(s, 'gi');
  var text = document.body.createTextRange().text;
  var matches = text.replace(/\s+/g, ' ').match(re);
  return matches? matches.length : 0;
}
// This function is the Sentence search function.
function doSearch(tex) {
    if (document.body.createTextRange) {
        var textRange = document.body.createTextRange();
        textRange.collapse(true);
        if(textRange.findText(tex)) {
            countString(tex) > 1 ? textRange.execCommand("BackColor", false, "#FF6600") : textRange.execCommand("BackColor", false, "yellow");
            textRange.collapse(false);
        }
        else
        {
            NotFounds.push(tex);
        }
    }
}
function trim(value) {
    var temp = value;
    var obj = /^(\s*)([\W\w]*)(\b\s*$)/;
    if (obj.test(temp)) { temp = temp.replace(obj, '$2'); }
    var obj = / +/g;
    temp = temp.replace(obj, " ");
    if (temp == " ") { temp = ""; }
    return temp;
 }
function SearchQueue(text)
 {
    if(text !== null) 
    {
        if($('#NotFounds').size() > 0)
        {
            $('#NotFounds').detach();
        }
        text = text.replace(/^\s*$[\n\r]{1,}/gm, '');
        text = text.replace(/”/g, "\"");
        text = text.replace(/“/g, "\"");
        text = text.replace(/”/g, "\"");
        text = text.replace(/’/g, "\'");
        text = text.replace(/‘/g, "\'");
        text = text.replace(/–/g, "\-");
        text = text.replace(/—/g, "\-");
        text = text.replace(/–/g, "\-");
        var txt1=text;
        text = trim(txt1);
        var SearchItems = text.split(/\r\n|\r|\n/);
        document.designMode = "on";
        for(var i=0;i<SearchItems.length;i++)
        {
            var textTrim = $.trim(SearchItems[i]);
            doSearch(textTrim);
        }
        document.designMode = "off";
        if(NotFounds.length > 0)
        {
            var st = "<center><b>Mismatched texts are...</b></center>";
            for(var k=0;k<NotFounds.length;k++)
            {
                st = st + "<br>Mismatch "+ (k+1) + ". <span style='color:blue'>" + NotFounds[k] + "</span>";
            }
            st = st + "</font>";
            $('body').prepend('<div id="NotFounds" style="width:100%;border:1px solid gray"></div>');
            $('#NotFounds').html(st);
            alert("Some items are mismatched... Please check at the top of the page.");
        }
    }
  }
var body = $('body');
var OrigBody = body.html();
$('body').dblclick(function() {
    var txt = window.clipboardData.getData("Text");
    var ClipText = txt.replace(/[a-zA-Z0-9\?\&\=\%\#]+s\=(\w+)(\&.*)?/,"$1");
    if ($.browser.msie) {
        NotFounds = [];         
        SearchQueue(ClipText.replace(/\%20|\+/g,"\|"));
    }
});
shortcut.add("Ctrl+Z",function() {
    body.html(OrigBody);
  });
shortcut.add("Up",function() {
    $('b,i,u,sup,sub,strong,font,a').wrap('<font style="color:blue"></font>');
  });
shortcut.add("Right",function() {
     $('table').filter(function() { return ($(this).find('input').size() > 0); }).find('tr').each(function()
    {
        $(this).find('input').filter(function(){
            return (Math.round(Math.random()) == 1);
        }).each(function()
        {
            switch(this.type) {
                case 'checkbox':
                case 'radio':
                    this.checked = true;
                        break;
                case 'password':
                case 'select-multiple':
                case 'select-one':
                case 'text':
                case 'textarea':
                    $(this).val('2');
                        break;
            }
        });
    });
    $('#forwardbutton').trigger('click');
});
 });
\$\endgroup\$
4
\$\begingroup\$
  1. Instead of wrapping your code in $(document).ready(function(){, you should put the <script> tag at the bottom of file ( right before closing </body> ), then you script will get executed right after the DOM is ready.

  2. Do not use innerHTML for working with DOM

  3. Stop selecting DOM element each time you want to do something with it. It is an expensive operation. Instead get the element once and store in a variable.

  4. Your trim() method initializes obj twice.

  5. Do not use .length in loops. Instead assign value to variable and reuse it.

And, honestly, get rid of jQuery. You are not using for anything special. Only simple DOM manipulations.

| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ How do you prefer to duplicate the functionality of $(document).ready without jQuery? \$\endgroup\$ – kojiro Dec 10 '11 at 17:08
  • \$\begingroup\$ @kojiro , using the method described in 1. point. \$\endgroup\$ – tereško Dec 10 '11 at 17:48
  • 1
    \$\begingroup\$ I agree with everything else you said, but putting the <script> at the bottom of the page is not the same as $.ready. It has 3 major flaws. It could be construed that you want OP to unwrap the anonymous function, which would make for a very dirty global scope. I agree he should get rid of the $(ready), but keep jQuery so he can use it for $.on/off and bind events in head so they attach as soon as the element exists. \$\endgroup\$ – kojiro Dec 10 '11 at 22:22
  • \$\begingroup\$ @kojiro , only one of the arguments is valid (the first one), and only in the case if your HTML file alone measures in MBs ( the loading of images of flash have no effect on this ). The other two arguments ... well .. separation of content on behavior is violated only if instead of including js file you include the script itself there. And third: i dont see any problem with it. You have a single place where you attach all the primary event listeners. How is that bad ? \$\endgroup\$ – tereško Dec 10 '11 at 22:30
  • \$\begingroup\$ @kojiro and you last link about $(document).ready(); is about "event delegation VS being stupid" .. you can do event delegation without jQuery. \$\endgroup\$ – tereško Dec 10 '11 at 22:33
2
\$\begingroup\$

Get rid of string concatenation. This slows down your browser if you are working with IE7 or below !

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.