1
\$\begingroup\$

The intent is to update an existing Drupal web page with content based on text entered into a textbox.

The existing page displays data from a database in a table. This code gets JSON formatted data, compares it to data in the table, and appends pertinent data in a hidden "td".

After doing this, the user may enter text into a textbox to search the table. The code uses that search text to find matches, filter out the rows that don't match, and add matching text to "td"s that contain the link to the file that contains the matching text.

It functions exactly as intended, but feels quite verbose. Can I make this more efficient or am I "reinventing the wheel"?

//append empty <th>
jQuery('div.dc-landing-content table thead tr').append('<th class="dc-pdftext" style="display:none;"></th>');

//declare url for data access
var url = "http://localhost/hosted/dj.json";//<--insert correct url here

//initialize pdfdata variable
var pdfdata = null;

//initialize toappend
var toappend = "";

//initialize var for tr
var selectedtr = null;

//initialize tdText
var tdText = "";

//initialize wordsArray
var wordsArray = null;

//initialize sMatch
var sMatch = "";

//wait for doc ready and run call functions
jQuery(document).ready(function() {

    pdfAppend_getDataAndRender(url);

    pdfAppend_searchResultInlineAppend();
});

function pdfAppend_getDataAndRender(url) {

    //get json data from the webservice endpoint
    jQuery.ajax({
        url: url,
        dataType: "json",
        success: function (currentData) {

            JSON.parse(JSON.stringify(currentData));

            pdfdata = currentData;

            pdfAppend_append();

        }
    });
}

function pdfAppend_append(){

    //iterate through each json object's data
    jQuery.each(pdfdata, function(i, item){

        if(item.id !== null && item.id !== "") {

            //determine dates for placing text match under correct calendar result
            var todayDate = jQuery("th.dc-current-date").text().replace(/Today/g, '').split(' ')[1];

            todayDate = todayDate.replace(/[()]/g, '');

            var nextDate = jQuery.trim(jQuery("th.dc-next-date").text()).replace(/Next Day/g, '').split(' ')[1];

            nextDate = nextDate.replace(/[()]/g, '');

            //determine date of item, attach the date, and append the item's text to the table in a hidden cell 
            if(item.date == todayDate){

                toappend = "<td class=\"dc-pdftext-current\" style=\"display:none;\">"+item.text+" "+item.date+"</td>";

                selectedtr = jQuery('a[href*="'+item.id+'"]').closest("tr");

                selectedtr.append(toappend);

                toappend = "";
            }else if(item.date == nextDate){

                toappend = "<td class=\"dc-pdftext-next\" style=\"display:none;\">"+item.text+" "+item.date+"</td>";

                selectedtr = jQuery('a[href*="'+item.id+'"]').closest("tr");

                selectedtr.append(toappend);

                toappend = "";
            }else{

                toappend = "<td class=\"dc-pdftext\" style=\"display:none;\">"+item.text+"</td>";

                selectedtr = jQuery('a[href*="'+item.id+'"]').closest("tr");

                selectedtr.append(toappend);

                toappend = "";
            }

        }
    });

}

function pdfAppend_searchResultInlineAppend(){

    jQuery('#item-lookup').keyup(function(){

        //row filter, removes rows that don't start with search match
        var jQueryrows = jQuery('div.dc-landing-content table tbody tr');

        //reset appended text match if search text is deleted
        if(jQuery(this).val().length < 3){

            jQuery(".pdf_Match").remove();

            jQueryrows.show();

        }

        //only search if entry is longer than 2 characters
        if(jQuery(this).val().length > 2){

            //reset search results for changes
            jQuery(".pdf_Match").remove();

            //get all 'td's minus judge name and category
            var jQuerycells = jQuery('div.dc-landing-content table tbody tr td').not('.dc-judge-value').not('.dc-judge-category');

            //get search text entry and remove spaces
            var searchText = jQuery(this).val().replace(/ +/g, '').toLowerCase();

            //initialize search text array for multi-word entry
            var searchArray = jQuery(this).val().toLowerCase().split(" ");

            //initialize matchFound boolean
            var matchFound = false;

            //reset rows on keyup
            jQueryrows.show();

            //row filter
            jQueryrows.each(function(){

                tdText = "";

                jQuery(this).find('td').each(function(i, item){

                    //add td text to variable containing all row td text
                    tdText += " " + item.innerText;

                    //trim the text
                    tdText = jQuery.trim(tdText).replace(/ +/g, ' ').toLowerCase();
                });

                //null wordsArray
                wordsArray = null;

                //split row text by line and spaces and store in array
                wordsArray = tdText.split(/[\s]+/g);

                //reset matchFound
                matchFound = false;

                //check for match
                for(var i=0; i<wordsArray.length; i++){

                    var numWords = "";

                    if(wordsArray[i].startsWith(searchText)){

                        matchFound = true;

                    }else if(searchArray.length > 1){

                        if(searchArray[0].startsWith(wordsArray[i])){

                            //create string of words from doc equal to the length of the search
                            for(var len=0; len<searchArray.length; len++){

                                numWords += wordsArray[i+len] + " ";

                                alert(numWords)
                            }

                            //compare word string to search string
                            if(numWords.startsWith(jQuery('#item-lookup').val())){

                                matchFound = true;
                            }
                        }
                    }
                }

                if(!matchFound){

                    jQuery(this).hide();
                }
            })

            //iterate through each cell and get text; must determine which cell the text came from in order to append to correct cell
            jQuerycells.each(function(i, item){

                tdText = item.innerText;

                //trim the text
                tdText = jQuery.trim(tdText).replace(/ +/g, ' ').toLowerCase();

                //null wordsArray
                wordsArray = null;

                //split cell text by line and spaces and store in array
                wordsArray = tdText.split(/[\s]+/g);

                //iterate through array
                for (var j=0; j<wordsArray.length; j++) {

                    //reset sMatch
                    sMatch = "";

                    //check array element for search entry
                    if (wordsArray[j].startsWith(searchText)){

                        //save match
                        sMatch = wordsArray[j];

                        //define toappend
                        toappend = "<ul class=\"pdf_Match\" style=\"list-style-type:none\"><li style=\"color: #8D8D8D\"><i><strong>"+sMatch+"</strong></i></li></ul>";

                        //check for existing result
                        if(!jQuery(item).hasClass('pdf_Match')){

                            //check if current date match
                            if(item.className == "dc-pdftext-current"){

                                //append results
                                jQuery(item).siblings(".dc-current-date-value").append(toappend);

                                //reset toappend
                                toappend = "";

                                //break from for loop
                                j = wordsArray.length;

                            }
                            //check if next date match
                            else if(item.className == "dc-pdftext-next"){

                                //append results
                                jQuery(item).siblings(".dc-next-date-value").append(toappend);

                                //reset toappend
                                toappend = "";

                                //break from for loop
                                j = wordsArray.length;

                            }else{

                                //append results
                                jQuery(item).append(toappend);

                                //reset toappend
                                toappend = "";

                                //break from for loop
                                j = wordsArray.length;
                            }
                        }
                    }
                    //multi-word search using arrays, but otherwise as above
                    else if(searchArray.length > 1){

                        var numWords = "";

                        //create string of words from doc equal to the length of the search
                        for(var len=0; len<searchArray.length; len++){

                            numWords += wordsArray[j+len] + " ";
                        }

                        //compare word string to search string
                        if(numWords.startsWith(jQuery('#item-lookup').val())){

                            //create string of matching phrases
                            for(var len=0; len<searchArray.length; len++){

                                if(wordsArray[len] !== undefined){

                                    sMatch += wordsArray[j+len]+" ";
                                }
                            }

                            toappend = "<ul class=\"pdf_Match\" style=\"list-style-type:none\"><li style=\"color: #8D8D8D\"><i><strong>"+sMatch+"</strong></i></li></ul>";

                            if(!jQuery(item).hasClass('pdf_Match')){

                                if(item.className == "dc-pdftext-current"){

                                    jQuery(item).siblings(".dc-current-date-value").append(toappend);

                                    toappend = "";

                                    j = wordsArray.length;

                                }else if(item.className == "dc-pdftext-next"){

                                    jQuery(item).siblings(".dc-next-date-value").append(toappend);

                                    toappend = "";

                                    j = wordsArray.length;

                                }else{

                                    jQuery(item).append(toappend);

                                    toappend = "";

                                    j = wordsArray.length;
                                }
                            }
                        }
                    }
                }
            });
        }
    });
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Welcome to CR! "this code" makes a pretty generic all-purpose title that pretty much every single question on this site could have. Please edit to instead tell us what your code is doing. \$\endgroup\$ – Mathieu Guindon Oct 7 '16 at 23:21
1
\$\begingroup\$

First, you're doing Drupal? Then this isn't The Drupal Way™.

With Drupal...

  • You normally display search data using Views.
  • To integrate a non-Drupal database into Views, a quick search led me to Views Database Connector.
  • The text search is just an exposed filter, but Drupal's built in exposed filters suck. Augment Drupal's exposed filter functionality with Better Exposed Filters.
  • Any custom logic you need to do, do it in a custom module. Use Drupal core hooks or Views hooks to alter everything like queries, returned data, rendering, data augments etc.
  • Any custom markup you need to alter, you can use views theme suggestions. Templates currently used by your view is found in the view's config, under Advanced -> Theme Information.

The whole idea of The Drupal Way™ is to keep configuration and rendering managed by Drupal, and that no magic is happening in front of Drupal. JavaScript suddenly altering what is expected to render is one of the hardest things to hunt down in Drupal. When things go wrong, there are so many things that could have caused it (themes, custom modules, misbehaving third-party modules, configuration, actual bugs, real limitations etc.).


If you have to do it this way (because yeah, deadlines suck and we have to do this the fastest way possible)...

Your code doesn't appear to be written in a Drupal-managed JavaScript. For custom logic like this, consider putting it in a module. That way, you can easily deploy this code, and have it be enabled or disabled at will. JavaScript is easily added using a module with a simple call to drupal_add_js.

Also, wrap your JS in Drupal Behaviors. One perk when you use this is that it doubles as $(document).ready(...). Another is that you can actually pass in settings and data from PHP into your JS, keeping your JS clean of hard-coded values.

One of the things I use this for is to construct Mustache templates using the Heredoc syntax in PHP (multi-line strings), pass them down to the JS and construct HTML from it.

Another is to have PHP do my web calls for me, bypassing the Same-Origin Policy. I pass the data down to the JS and all it does is do some logic and render.

\$\endgroup\$
0
\$\begingroup\$

There are lots of things to improve, but I'll show just one function deduped:

function pdfAppend_append(pdfdata) {
    function extractDate(selector) {
        return $(selector).text().trim()
            .replace(/Today|Next day/g, '').split(' ')[1]
            .replace(/[()]/g, '');
    }
    $.each(pdfdata, function(i, item) {
        if (item.id) {
            //determine dates for placing text match under correct calendar result
            //attach the date, and append the item's text to the table in a hidden cell 
            var tdClass = item.date == extractDate('th.dc-current-date') ? '-current' :
                          item.date == extractDate('th.dc-next-date') ? '-next' :
                          '';
            $('a[href*="' + item.id + '"]')
                .closest('tr')
                .append('<td class="dc-pdftext' + tdClass + '" style="display:none;">' + 
                         item.text + '</td>');
        }
    });
}

In short:

  1. Don't use global variables for things that make sense only in local code context
  2. Use single quotes for strings so that doublequotes can be easily used for html attributes
  3. Instead of huge, vague, nested if-else, simply return from a function
  4. Deduplicate the code, DRY, pdfAppend_searchResultInlineAppend can be halved easily.
  5. Don't use self-evident comments like // initialize the variable
\$\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.