-1
\$\begingroup\$

Note:

  1. I am very new to Javascript.
  2. The code I provided is long but not complicated.

The code below works and runs fine. But I would like to seperate it out into logical 'pure' functions to make it clearer whats going on. I started to re-factor this myself and made some good progress, however, I quickly started to return to problems with dependencies and embedded functions.

I'd be interested to see how others would write the following:

   var contactsController = {

fetch: function()
{
    // Parses the returned JSON into the database
    var fetchSuccess = function(json)
    {          
        var NAME = "Contact",
            COLUMNS = ['Name', 'GivenNames', 'FamilyName', 'Telephone'];

        if (json.Response.Code == 0)
        {
            database.open();
            database.dropTable(NAME);
            database.createTable(NAME, COLUMNS);    

            // .Contact is an array of contact objects
            var contacts = json.Contacts.Contact; 

            for (var contact in contacts)
            {
                var contact = contacts[contact];
                database.insert(NAME, COLUMNS, [contact.Name, contact.GivenNames, contact.FamilyName, contact.Telephone] );
            }

            contactsController.populateList();
        }
    }

    ServerConn.fetchContacts(fetchSuccess);
},

populateList: function()
{
    var $page = $("#contactsPage");

    $page.find(".content").empty();
    $page.find(".content").html("<ul data-role='listview' data-filter='true'><\/ul>");
    $list = $page.find(".content ul");

    var render = function(character)
    {
        return function(tx, result)
        {
            var max = result.rows.length;

            if (max > 0)
            {
                var header = "<li data-role='list-divider'>" + character + "</li>",
                    listItem = $(header);

                $list.append(header);

                for (var i = 0; i < result.rows.length; i++) 
                {
                    var contact = result.rows.item(i),
                        strHtml = "<li><a href='#contactPage'>" + contact.Name + "<\/a><\/li>";

                    listItem = $(strHtml);
                    $list.append(listItem);

                    // store the data in the DOM
                    $list.find("a:last").data("contactObj", contact);
                }

                $list.find("a").click(function()
                {
                    var $this = $(this);
                    $("#contactPage").data("contactObj", $this.data("contactObj"));
                });

                $list.listview(); //Only fires once?
                $list.listview('refresh');  
            }
        }
    }

    var str = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
    for(var i = 0; i < str.length; i++)
    {
        var sql = "SELECT * FROM Contact WHERE Exclude = ? AND Name LIKE ? ORDER BY Name",
            nextChar = str.charAt(i),
            selargs =  ['0', nextChar + "%"];

        database.open();
        database.query(sql, selargs, render(nextChar));
    }
}

};

\$\endgroup\$
0

1 Answer 1

2
\$\begingroup\$

A lot of the logic in your code belongs to a template, not directly in code. You can find tons of templating engines for javascript or make your own.

An example of a template that could be used here is:

<script type="text/html" id="letter-index-view">
@if( model.result.rows.length ) {
    <li data-role="list-divider">@model.result.character</li>
    @foreach( contact in model.result.rows ) {
        <li><a href="#contactPage" id="@contact.ContactID">@contact.Name</a></li>
    }
}
</script>

Now populateList becomes this:

populateList: function()
    {
        var $page = $("#contactsPage"),
            tmpl = Template.getById("letter-index-view"),
            str = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";

        $list = $("<ul>", {
            "data-role": "listview",
            "data-filter": "true"
        });

        $list.on( "click", "a", function() {
            $("#contactPage").data("contactId", this.id);
        });

        $page.find(".content").empty().append( $list );

        for(var i = 0; i < str.length; i++)
        {
            var nextChar = str.charAt(i),
                sql = "SELECT *, ? AS character FROM Contact WHERE Exclude = ? AND Name LIKE ? ORDER BY Name",
                selargs =  [nextChar, '0', nextChar + "%"];

            database.open();
            database.query(sql, selargs, function(tx, result){
                $list.append( tmpl( {
                    result: result
                }));

                $list.listview(); //Only fires once?
                $list.listview('refresh');  
            });
        }
    }

Note that I made some other changes as well, since template is kind of static html it cannot have object references like $list.find("a:last").data("contactObj", contact); so the links are uniquely identified with their database row id.

I also modified the query to return the character used in that query so a static function can be used for the query callback instead of 26 different ones.

Another reason for populateList being lengthy is that it's doing view and model stuff at the same time. You should move all the DOM stuff into a view.

\$\endgroup\$
2
  • \$\begingroup\$ There are some really nice bits of code here. I particularly like storing the character in the SQL rather than using the immediately returning function. Is the template from scratch or have you used a framework? \$\endgroup\$
    – JonWells
    Apr 11, 2012 at 11:15
  • \$\begingroup\$ @CrimsonChin Not sure what you mean... I just wrote the template according to your code. A template engine would convert that to real javascript function (Template.getById("letter-index-view")) which can be called to generate HTML according to the templating language used. \$\endgroup\$
    – Esailija
    Apr 11, 2012 at 11:39

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