Multiple select list results in order - cont'd


#42

Reading the JSON config file is easiest solution for what we want to do here.

The config file that we’re going to use is in config/categories.json

config/categories.json

[
  {
    "title": "Fruits",
    "items": [
      "Apples",
      "Blackberries",
      "Raspberries"
    ]
  },
  {
    ...
  }
]

We’ll want to use a separate createcategories.js module, which can be made by using tests to help drive the direction of the code.

tests/main.js

    require(
        [
            "../tests/filtercheckboxesbytext.spec",
            ...
            "../tests/createcategories.spec"
        ],

The TestRunner tells us that the createCategories test doesn’t exist, so we should create that.

createcategories.spec.js

/*jslint browser */
/*global define, describe, it, expect */
define(["json!categories", "createcategories"], function (catData, createCategories) {
});

Reading the categories.json file is the next issue.

We can easily read and parse the JSON file using a require-json plugin that’s obtained from https://github.com/millermedeiros/requirejs-plugins

We just need lib/text.js and src/json.js from there, both of which we can place in our lib/require/plugins/ folder.

We can then tell the main.js file for the tests and for the code where to find them.

tests/main.js

    paths: {
        "text": ["../lib/require/plugins/text"],
        "json": ["../lib/require/plugins/json"],
        "categories": "../tests/categories.json",
        ...
    },

src/main.js

    paths: {
        "text": ["../lib/require/plugins/text"],
        "json": ["../lib/require/plugins/json"],
        ...
    },

The tests need to use a separate categories.json file, as the one that the ordinary code uses will change and we still want to keep the tests working. Because of that, the config/categories.json file is copied to tests/categories.json, and we tell the code where to find the json file.

tests/main.js

    paths: {
        "categories": "../tests/categories.json",
        ...
    },

tests/main.js

    paths: {
        "categories": "../../config/categories.json",
        ...
    },

And with that tests return back to passing.

Testing with categories.json

We now want to create the createCategories.init function and test that the createCategories init code gets run:

createcategories.json.js

        it("is initialized from the search results code", function () {
            spyOn(createCategories, "init");
            searchResults.init(catData);
            expect(createCategories.init).toHaveBeenCalled();
        });

We need to update the searchresults code, first to move the existing init code to a separate function:

searchresults.js

        function initSearchResults() {
            $(categories).filterCheckboxesByText($(searchField));
            ...
        }
        return {
            init: function () {
                initSearchResults();
            }
        };

This lets us easily call the createCategories function.

searchresults.js

define(
    [
        "createcategories",
        "jquery",
        ...
    ],
    function (createCategories, $, checkboxList, resultsController) {
    ...
        return {
            init: function (catData) {
                createCategories.init(catData);
                initSearchResults();
            }
        };

which needs us to create the createCategories init function:

createcategories.js

/*jslint browser */
/*global define */
define([], function () {
    "use strict";
    function init(catData) {
        console.log(catData);
    }
    return {
        init: init
    };
});

The moment of truth

We can then require the categories file and use it.

src/main.js

require(["json!categories", "searchresults"], function (catData, searchResults) {
    "use strict";
    searchResults.init(catData);
});

Which shows the contents of the categories JSON object in the console browser.

Creating a place for the categories

When we have category data, we want to remove any existing categories below the search area, and create new ones from the category data.

Currently though, the search area is called mytextbox, which isn’t a good name. It would be much better to have a name such as categorysearch instead.

index.html

    <!--<p><input type="text" id="mytextbox" placeholder="Search Foods" autocomplete="off"></p>-->
    <p><input type="text" id="categorysearch" placeholder="Search Foods" autocomplete="off"></p>

searchresults.js
// var searchField = “#mytextbox”;
var searchField = “#categorysearch”;


createsandbox.js
```javascript
            html: "<p><input type='text' id='categorysearch'></p>" +

Adding a filter test

While making this change I decided to find out what happens if I misspell categorysearch, and the tests all still pass. That is not good.

searchresults.js

        var searchField = "#badname";

We need to add a test to catch such an obvious issue.

searchresults.spec.js

        it("uses a search filter", function () {
            categorysearch = $("#categorysearch");
            categorysearch.val("fru");
            expect($("#categories p:visible").length).toBe(2);
            $(categorysearch).trigger("keyup");
            expect($("#categories p:visible").length).toBe(1);
        });

That test fails, as expected, and when we give the searchfield a name that matches the category search input field, the test passes.

searchresults.js

        var searchField = "#categorysearch";

Remove old categories before adding new ones

Next up we can remove any category sections before adding them in using the category data.

createcategories.spec.js

        it("removes the categories section", function () {
            document.body.innerHTML += "<div id='categories'>";
            categories = document.querySelector("#categories");
            searchResults.init(catData);
            categories = document.querySelector("#categories");
            expect(categories).toBeNull();
        });

We can make that pass just by removing the element:

createcategories.js

    function init() {
        categories = document.querySelector("#categories");
        categories.remove();
    }

and it does pass when that’s the only test that runs, but other tests running before it have side-effects that result in the test failing because more than one categories element exists.

Fixing side-effects

Removing the other tests from the tests/main.js file, all of the createcategories.spec.js tests work, so it’s time to remove the side-effect from other tests.

Removing the sandbox after all of filtercheckboxes tests results in success here.

filtercheckboxes.spec.js

        afterAll(function () {
            sandbox.init({html: ""});
        });

The same afterAll function is also added to the scrollmanager.spec.js and checkboxlist.spec.js tests.

After that, we’re left with a few other tests that remain to be fixed. The Search results tests are the ones that fails, which we can make pass by returning from the createcategories init function if there is no catData.

createcategories.js

    function init(catData) {
        var categories = document.querySelector("#categories");
        if (!catData) {
            return;
        }
        categories.remove();
    }

But if there are no categories, that code’s going to error, so we need to account for when there are no categories as well.

createcategories.spec.js

        it("doesn't error when categories doesn't exist", function () {
            categories = document.querySelector("#categories");
            expect(categories).toBeNull();
            searchResults.init(catData);
            categories = document.querySelector("#categories");
        });

createcategories.js

    function init(catData) {
        ...
        if (categories) {
            categories.remove();
        }
        categories.remove();
    }

And all of the tests are passing once more.

Future tests to do later

Working through the above situations helps me to understand that there are three situations in which I want the code to work:

  • categories in HTML and no category data file
  • no categories and category data file
  • categories in HTML replaced with category data file

Instead of creating tests for all of those situations, I’ll put a pin in them until the createcategories code is done, then revisit them and add any needed tests to cover missing features.

createcategories.spec.js

        xit("works in a variety of common situations", function () {
            // TODO: there are three situations in which I want the code to work:
            // * categories in HTML and no category data file
            // * no categories and category data file
            // * categories in HTML replaced with category data file
        });

Adding categories to the page

Where do we go from here? Do we loop through each category item and add it to the page? Do we add a single item? Do we ignore the items and do something else?

We can use our overall goal to help drive the code. We need to create the following HTML code for the categories:

    <div id="categories">
      <p>
        <input type="checkbox" id="chkFruits" value="Fruits">
        <label for="chkFruits">Fruits</label>
      </p>
      <p>
        <input type="checkbox" id="chkVegetables" value="Vegetables">
        <label for="chkVegetables">Vegetables</label>
      </p>
      <p>
        <input type="checkbox" id="chkNuts" value="Nuts">
        <label for="chkNuts">Nuts</label>
      </p>
      <p>
        <input type="checkbox" id="chkMeats" value="Meats">
        <label for="chkMeats">Meats</label>
      </p>
    </div>

With that HTML structure in mind, we can start from the outside and work our way in. So first, the categories element needs to be added below the categorysearch input field.

The createcategories code doesn’t need to worry about adding the categorysearch field. That’s outside of the scope of the createcategories code. Instead, passing the categorysearch field into the init function makes a lot of sense, so that we can deal with that issue later on before passing it in.

createcategories.js

    function init(catData, search) {
        ...
    }

Which means updating the searchresults code to give it the search field.

            init: function (catData) {
                var search = document.querySelector(searchField);
                createCategories.init(catData, search);
                initSearchResults();
            }

To test that the paragraph elements exist in the categories section, we need to start with a categorysearch element, and being the good tidy people that we are we clean up after ourselves after each test.

createcategories.spec.js

    describe("Create categories", function () {
        var search;
        beforeEach(function () {
            document.body.innerHTML += "<input id='categorysearch'>";
            search = document.querySelector("#categorysearch");
        });
        afterEach(function () {
            search = document.querySelector("#categorysearch");
            search.remove();
        });
        ...
        it("adds paragraphs for each category", function () {
          // yet to be implemented
        });

We can now test that the categories section is added below the search field.

createcategories.spec.js

        it("adds the categories section to the search field", function () {
            searchResults.init(catData);
            expect(search.nextSibling.id).toBe("categories");
        });

createcategories.js

    function init(catData, search) {
        ...
        categories = document.createElement("div");
        categories.id = "categories";
        search.parentNode.appendChild(categories, search.nextSibling);
    }

That causes some other tests to fail though. We need to ensure that we don’t guard against situations when the search element doesn’t exist.

createcategories.js

        // if (!catData) {
        if (!catData || !search) {
            return;
        }

And the test that checks that a categories element is removed, needs to be updated. Instead of checking that it’s removed, we can check that the categories element no longer has a parent node.

createcategories.spec.js

        it("removes pre-existing categories sections", function () {
            document.body.innerHTML += "<div id='categories'>";
            categories = document.querySelector("#categories");
            expect(categories.parentNode).not.toBeNull();
            searchResults.init(catData);
            // expect(categories).toBeNull();
            expect(categories.parentNode).toBeNull();
        });

To fix the other failing tests, the categories need to be cleaned up at the end of each test too.

createcategories.spec.js

    describe("Create categories", function () {
        var search;
        var categories;
        ...
        afterEach(function () {
            search = document.querySelector("#categorysearch");
            search.remove();
            categories = document.querySelector("#categories");
            if (categories) {
                categories.remove();
            }
        });

And all of the tests return to passing, which is a relief.

Refactor to improve the code

Now that we have passing tests we can refactor the code. The process of removing an old categories section and adding a new one, we should move out to a separate function. I don’t know what to call it yet though, so I’ll just call it replaceOldWithNew for now.

    function replaceOldWithNew(categories, search) {
        if (categories) {
            categories.remove();
        }
        categories = document.createElement("div");
        categories.id = "categories";
        search.parentNode.appendChild(categories, search.nextSibling);
    }
    function init(catData, search) {
        var categories = document.querySelector("#categories");
        if (!catData || !search) {
            return;
        }
        // if (categories) {
        //     categories.remove();
        // }
        // categories = document.createElement("div");
        // categories.id = "categories";
        // search.parentNode.appendChild(categories, search.nextSibling);
        replaceOldWithNew(categories, search);
    }

The replaceOldWithNew function name isn’t a good name, so we can now focus on what else to call it.

Terms like refresh or recreate aren’t suitable. How about replenish? No. When you have old sugar and replace it with new, you reprovision? We’re closing in on the appropriate term. It’s one of the hardest programming jobs to come up with the correct names.

Renew! That’s our word.

createcategories.js

    // function replaceOldWithNew(categories, search) {
    function renewCategories(categories, search) {
        ...
    }
    ...
            // replaceOldWithNew(categories);
            renewCategories(categories);

Now that we have a new categories section being added, this is a good time to take a break.

I’ll follow up with the rest of the process of using the JSON data to create categories, followed by the results section.


#43

We’ve renewed the categories area, so now it’s time to add something in there.
Today we’re going to be dealing with:

  • item codes
  • adding paragraphs
  • adding input and label
  • fixing side-effects
  • Refactoring the tests
  • Add the label
  • Add id and for
  • Avoiding duplicate id’s

Item codes

As a reminder, here’s some of the JSON data that we’re using:

categories.json

[
  {
    "title": "Fruits",
    "items": [
      "Apples",
      "Blackberries",
      "Raspberries"
    ]
  },
  ...
]

Additionally, you are wanting to assign id codes to each item, so we need to update the categories.json data to support those id codes.

categories.json

[
  {
    "title": "Fruits",
    "items": [
      {"name": "Apples", "code": "1001"},
      {"name": "Blackberries", "code": "289"},
      {"name": "Raspberries", "code": "rasp-1234"}
    ]
  },
  ...
]

Because the code isn’t always numbers, we are using strings for the item code. We’ll come back to using those later.

We want to end up with:

<div id="categories">
  <p>
    <input id="chkFruits" type="checkbox" value="Fruits">
    <label for="chkFruits">Fruits</label>
  </p>
  ...
</div>

So first, we’ll add the paragraphs.

Adding paragraphs

With four different categories, we can expect for there to be four paragraphs in the categories section.

createcategories.spec.js

        it("adds paragraphs for each category", function () {
            searchResults.init(catData);
            categories = document.querySelector("#categories");
            expect(categories.querySelectorAll("p").length).toBe(4);
        });

createcategories.js

    function addCategoryItems(catData) {
        return catData.map(function () {
            return document.createElement("p");
        }).reduce(function (container, el) {
            container.appendChild(el);
            return container;
        }, document.createDocumentFragment());
    }
    ...
        renewCategories(categories, search);
        categories = document.querySelector("#categories");
        var items = addCategoryItems(catData);
        categories.appendChild(items);

With all tests passing we can refactor the code to make it easier to understand.

createcategories.js

        function makePara() {
            return document.createElement("p");
        }
        function combine(container, el) {
            container.appendChild(el);
            return container;
        }
        return catData
            .map(makePara)
            .reduce(combine, document.createDocumentFragment());

Adding input and label

With the input and label, I don’t want to use the item name because they can contain characters that are not valid to use as identifiers.

One solution is to use no identifier and just wrap the label around the input, avoiding the need for id and for attributes.

    <label>
        <input type="checkbox" value="Fruits">
        Fruits
    </label>

But, this is not compatible with screen-reading software so we need to use id and for attributes.
The id and for attributes are not for human consumption, so we can just use an increasing index value for them.

        <input id="chk2" type="checkbox" value="Fruits">
        <label for="chk2">Fruits</label>

We can use a test that checks that there are four input elements with one in the first paragraph with appropriate type and value attributes.

This can seem to be complex, but tests help to make things so much simpler to develop.

createcategories.spec.js

        it("adds an input to each paragraph", function () {
            searchResults.init(catData);
            categories = document.querySelector("#categories");
            expect(categories.querySelectorAll("input").length).toBe(4);
        });
        it("has an input with correct type and value", function () {
            var input = categories.querySelector("p input");
            var name = catData[0].title;
            expect(input.getAttribute("type")).toBe("checkbox");
            expect(input.getAttribute("value")).toBe(name);
        });

The first test that expects 0 to be 4, we can make pass by adding an input to each paragraph.

createcategories.js

        function addCategory(p) {
            var input = document.createElement("input");
            p.appendChild(input);
            return p;
        }
        return categories
            .map(makePara)
            .map(addCategory)
            ...

The other test expects null to be “checkbox”

createcategories.js

        function addCategory(p) {
            var input = document.createElement("input");
            input.type = "checkbox";
            p.appendChild(input);
            return p;
        }

Which leaves the test that expects null to be “Fruits”. This we can deal with by getting the index:

        function addCategory(p, index) {
            ...
            input.setAttribute("value", catData[index].title);
            p.appendChild(input);
            ...
        }

But that’s not how I want the code to end up being. I want to refactor that code to improve it, but first I need all of the test to be passing.

categories section side-effect

One of the createcategories tests has a problem when there are other tests, that on further investigation comes from the searchresults tests. Emptying the sandbox fixes that problem.

searchresults.spec.js

        afterAll(function () {
            sandbox.init({html: ""});
        });

Improving createcategories inputs

Now that all of the tests are passing (important), I can move on to refactoring the add inputs code to make things better.

Instead of using an index value, I want instead to go down through the JSON data collecting values, and then go out building elements from those values.

Currently we have:

createcategories.js

        return catData
            .map(makePara)
            .map(addCategory)
            .reduce(combine, document.createDocumentFragment());

Instead of the above I want to:

  1. start with the category object information
  2. turn that into a list of category titles
  3. change each category title into an input
  4. put each input in a paragraph
  5. combine those paragraphs together

Which moves the code through the following transmations:

  1. [{"title": "Fruits", "items": {...}}, ...]
  2. ["Fruits", "...", ...]
  3. [<input type="checkbox" value="Fruits">, ...]
  4. [<p><input ...></p>, <p><input ...></p>, ...]
  5. <p><input ...></p> <p><input ...></p> ...

This is what’s called information flow, where each stage is then transformed into the next.

I can start by switching makePara and addCategory, so that they’re in the same order as our desired plan, renaming makePara to wrapWithPara.
The addCategory function just needs to get the title from the categoryInfo and return the input, and wrapWithPara just places whatever it receieves into a paragraph.

createcategories.js

        // function addCategory(p, index) {
        function addCategory(categoryInfo) {
            var input = document.createElement("input");
            input.setAttribute("type", "checkbox");
            // input.setAttribute("value", catData[index].title);
            input.setAttribute("value", categoryInfo.title);
            // p.appendChild(input);
            // return p;
            return input;
        }
        // function makePara() {
        function wrapWithPara(el) {
            // return document.createElement("p");
            var p = document.createElement("p");
            p.appendChild(el);
            return p;
        }
        ...
        return catData
            .map(addCategory)
            .map(wrapWithPara)
            .reduce(combine, document.createDocumentFragment());

We can now get the category titles, and complete this refactor by passing those titles to the addCategory function.

createcategories.js

        function getTitle(categoryInfo) {
            return categoryInfo.title;
        }
        // function addCategory(categoryInfo) {
        function addCategory(title) {
            var input = document.createElement("input");
            input.setAttribute("type", "checkbox");
            // input.setAttribute("value", categoryInfo.title);
            input.setAttribute("value", title);
            return input;
        }
        ...
        return catData
            .map(getTitle)
            .map(addCategory)
            .map(wrapWithPara)
            .reduce(combine, document.createDocumentFragment());

Refactor the tests

I’ve noticed that most of the createcategories tests involve initing searchresults and getting the categories section, so moving that into a different section and using beforeEach in there, should help to simplify things there.

createcategories.spec.js

    describe("Create categories", function () {
        ...
        describe("when init'd", function () {
            beforeEach(function () {
                searchResults.init(catData);
                categories = document.querySelector("#categories");
            });
            it("adds the categories section to the search field", function () {
                // searchResults.init(catData);
                // categories = document.querySelector("#categories");
                expect(search.nextSibling.id).toBe("categories");
            });
            ...
            it("has an input with correct type and value", function () {
                // searchResults.init(catData);
                // categories = document.querySelector("#categories");
                var input = categories.querySelector("p input");
                var name = catData[0].title;
                expect(input.getAttribute("type")).toBe("checkbox");
                expect(input.getAttribute("value")).toBe(name);
            });
        });
    });

Add the label

We can now easily add a label to the above code. The test is:

createcategories.spec.js

            it("has a label with a name", function () {
                var label = categories.querySelector("p label");
                var name = catData[0].title;
                expect(label.innerText).toBe(name);
            });

We can move the input code into its own separate function:

createcategories.js

        function categoryInput(title) {
            var input = document.createElement("input");
            input.setAttribute("type", "checkbox");
            input.setAttribute("value", title);
            return input;
        }
        function addCategory(title) {
            // var input = document.createElement("input");
            // input.setAttribute("type", "checkbox");
            // input.setAttribute("value", title);
            // return input;
            var category = document.createDocumentFragment();
            category.appendChild(categoryInput(title));
            return category;
        }

We can now easily add the label.

createcategories.js

        function categoryLabel(title) {
            var label = document.createElement("label");
            label.innerText = title;
            return label;
        }
        function addCategory(title) {
            var category = document.createDocumentFragment();
            category.appendChild(categoryInput(title));
            category.appendChild(categoryLabel(title));
            return category;
        }

Adding id and for

Next up we want to connect the input and label together with an id and for attribute.

createcategories.spec.js

            it("has matching id and for attributes", function () {
                var input = categories.querySelector("p input");
                var label = categories.querySelector("p label");
                var idAttr = input.getAttribute("id");
                var forAttr = label.getAttribute("for");
                expect(idAttr).toBe(forAttr);
            });

And that passes. That was easy, too easy.

I don’t want to check that undefined equals undefined. There must be a valid value, so I’ll also check that it starts with “chk”.

createcategories.spec.js

            it("has matching id and for attributes", function () {
                ...
                expect(idAttr.substr(0, 3)).toBe("chk");
                expect(idAttr).toBe(forAttr);
            });

That gives a better test error, of idAttr is null so we can add an id attribute to the input

createcategories.js

        function categoryInput(title) {
            var input = document.createElement("input");
            input.setAttribute("type", "checkbox");
            input.setAttribute("id", "chk1");
            ...
        }

The id is supposed to be different for each input, but we’ll get to that shortly.

The next test error is that it expects chk1 to be null, so we need to add a for attribute to the label too.

createcategories.js

        function categoryLabel(title) {
            var label = document.createElement("label");
            label.setAttribute("for", "chk1");
            ...
        }

That passes with the test, but that’s not good enough. Each id needs to be unique, so we can check that there’s only one of that id attribute.

createcategories.spec.js

            it("has matching id and for values", function () {
                ...
                expect(idAttr.substr(0, 3)).toBe("chk");
                expect(document.querySelectorAll("[id=" + idAttr + "]").length).toBe(1);
                expect(forAttr).toBe(idAttr);
            });

And we’re now given an error that 4 is expected to be 1.

To deal with that issue we need replace “chk1” with a function parameter.

createcategories.js

    // function categoryInput(title) {
    function categoryInput(title, idAttr) {
        ...
        // input.setAttribute("id", "chk1");
        input.setAttribute("id", idAttr);
        ...
    }
    // function categoryLabel(title) {
    function categoryLabel(title, forAttr) {
        ...
        label.setAttribute("for", forAttr);
        ...
        
    }
    function addCategory(title) {
        var category = document.createDocumentFragment();
        var id = "chk1";
        // category.appendChild(categoryInput(title));
        category.appendChild(categoryInput(title, id));
        // category.appendChild(categoryLabel(title));
        category.appendChild(categoryLabel(title, id));
        return category;
    }

And we can now replace “chk1” with a function call to create a unique identifier.

createcategories.js

    function makeId(prefix) {
        return prefix + "1";
    }
    function addCategory(title) {
        var category = document.createDocumentFragment();
        // var id = "chk1";
        var id = makeId("chk");
        ...
    }

I can now replace “1” with an increasing number, and ensure that it’s unique, otherwise get another one.

createcategories.js

    var idIndex = 0;
    ...
    function makeId(prefix) {
        // return prefix + "1";
        var id = prefix + idIndex;
        idIndex += 1;
        return id;
    }

Avoiding duplicate id’s

There’s only one other problem - what happens if the id already happens to exist on the page? We need a test to simulate this problem.
This test needs to be outside of the describe init'd section, because we need to add a conflicting identifier to the page before the code it inited.

createcategories.spec.js

        it("has matching id and for values", function () {
            document.body.innerHTML += "<div id='chk0'></div>";
            searchResults.init(catData);
            categories = document.querySelector("#categories");
            var input = categories.querySelector("p input");
            var idAttr = input.getAttribute("id");
            expect(document.querySelectorAll("[id=" + idAttr + "]").length).toBe(1);
        });

There’s a problem though, and logging the idAttr helps to reveal the problem.

createcategories.spec.js

        it("avoids id conflicts", function () {
            ...
            var idAttr = input.getAttribute("id");
            console.log(idAttr);
            expect(document.querySelectorAll("[id=" + idAttr + "]").length).toBe(1);
        });

From one test run to the next we get different values, such as “chk0”, “chk28”, “chk32”, “chk16”
We need the idIndex to be zero’d when the createcategories code is initialized.

createcategories.js

    function init(catData, search) {
        var categories = document.querySelector("#categories");
        idIndex = 0;
        ...
    }

And now, we have a properly failing test, saying that it expects 2 to be 1.

We need the makeId function to check if the id already exists, and if it does to then go around again to get the next id.

createcategories.js

    function makeId(prefix) {
        var id = prefix + idIndex;
        idIndex += 1;
        // return id;
        if (!document.getElementById(id)) {
            return id;
        }
        return makeId(prefix);
    }

The code passes, and the categories are all being generated (or regenerated) from the JSON data file.

Next time we’ll use the same techniques to generate the results section too, and the code can then be used to generate all of the categories and results.


#44

In the previous post I attempted to structure things so that there was a clear motivation for each change that occurred to the code. I’ll try to do the same here, but as the results section is more complex, I’ll do it with less detail or otherwise this will be a very long post indeed.

I’ll use a createResults section with the intention of duplicating some of the code. It’s okay to have some duplication, so long as you consider removing that duplication when it’s in three or more places.

createResults tests

I’ll want to pass the categories section to the createResults code, which means getting it from the createCategories code, which means having a test for adding a results section. That way the changes can be clearly motivated by what’s needed to make the test pass.

The createcategories file isn’t really a suitable place for the results code, so a new test file is needed for createresults

            "../tests/createcategories.spec",
            "../tests/createresults.spec"

The initial test ensures that the createresults.js file exists, and that an init function can be called.
We’ll also want to remove the difference sections after each test, to help clean things up.

createresults.spec.js

/*jslint browser*/
/*global define, describe, beforeEach, it, spyOn, expect */
define([
    "json!categories", "createresults", "searchresults"
], function (catData, createResults, searchResults) {
    "use strict";

    describe("Create results", function () {
        var categories;
        beforeEach(function () {
            document.body.innerHTML += "<input id='categorysearch'>";
        });
        afterEach(function () {
            function removeBySelector(selector) {
                document.querySelectorAll(selector).forEach(function (el) {
                    el.remove();
                });
            }
            removeBySelector("#categorysearch");
            removeBySelector("#categories");
            removeBySelector("#clearAll");
            removeBySelector("#results");
        });
        it("is initialized from the search results code", function () {
            spyOn(createResults, "init");
            searchResults.init();
            expect(createResults.init).toHaveBeenCalled();
        });
    });
});

Using tests to drive code development

The above test forces us to create the createresults.js file, which we can copy from the structure in createcategories.

createresults.js

/*jslint browser */
/*global define */
define([], function () {
    "use strict";

    function init(catData, categories) {

    }
    return {
        init: init
    };
});

And, the init code is called from the searchresults code, where we’ll want the category section to be returned from the createCategories code.

searchresults.js

define(
    [
        "createcategories",
        "createresults",
        ...
    ],
    ...
            init: function (catData) {
                var search = document.querySelector(searchField);
                // createCategories.init(catData, search);
                var categorySection = createCategories.init(catData, search);
                createResults.init(catData, categorySection);
                initSearchResults();
            }

And, we need the createCategories code to return the categories section.

createcategories.js

    function init(catData, search) {
        ...
        return categories;
    }

clearAll and results sections

I can now test that any pre-existing clearAll and results sections are removed, and that things don’t error if they aren’t there either.

createresults.spec.js

        it("removes pre-existing clearAll sections", function () {
            document.body.innerHTML += "<button id='clearAll'>Clear all results</button>";
            var clearAll = document.querySelector("#clearAll");
            expect(clearAll.parentNode).not.toBeNull();
            searchResults.init(catData);
            expect(clearAll.parentNode).toBeNull();
        });
        it("doesn't error when clearAll doesn't exist", function () {
            var clearAll = document.querySelector("#clearAll");
            expect(clearAll).toBeNull();
            searchResults.init(catData);
            clearAll = document.querySelector("#clearAll");
        });
        it("removes pre-existing results sections", function () {
            document.body.innerHTML += "<div id='results'></div>";
            var results = document.querySelector("#results");
            expect(results.parentNode).not.toBeNull();
            searchResults.init(catData);
            expect(results.parentNode).toBeNull();
        });
        it("doesn't error when results doesn't exist", function () {
            var results = document.querySelector("#results");
            expect(results).toBeNull();
            searchResults.init(catData);
            results = document.querySelector("#results");
        });
        describe("when init'd", function () {
            beforeEach(function () {
                searchResults.init(catData);
                categories = document.querySelector("#categories");
            });
            it("adds the clearAll section below the categories section", function () {
                expect(categories.nextSibling.id).toBe("clearAll");
            });
        });

And copy over and rename some of the createcategories code:

    function renewClearAll(clearAll, categories) {
        if (clearAll) {
            clearAll.remove();
        }
        clearAll = document.createElement("div");
        clearAll.id = "clearAll";
        categories.parentNode.appendChild(clearAll, categories.nextSibling);
        return clearAll;
    }
    function renewResults(results, clearAll) {
        if (results) {
            results.remove();
        }
        results = document.createElement("div");
        results.id = "results";
        clearAll.parentNode.appendChild(results, clearAll.nextSibling);
        return results;
    }
    function init(catData, categories) {
        var clearAll = document.querySelector("#clearAll");
        var results = document.querySelector("#results");
        if (!catData || !categories) {
            return;
        }
        clearAll = renewClearAll(clearAll, categories);
        results = renewResults(results, clearAll);
    }

I can now init the createresults code and work on adding the elements.

However, the above code has resulted in several tests randomly breaking from being tested in a random order. This occurs because some tests don’t clean up the environment well enough after some tests. I’ll get in to how we can clean these things up in the next post.


#45

Some of the tests conflict with each other because they leave things lying around afterwards, which because of the random ordering of tests, ends up conflicting with other tests.

The randomly ordered tests aren’t the problem here. Instead, they have helped to expose problems that we wouldn’t otherwise be able to see, and are are doing their job in helping us to have more resilient code.

The things that get done today are:

  • Reporting on the issue
  • Cleaning up sandbox code
  • Tidying up:
    • scrollmanager tests
    • searchresults tests
    • createcategories tests
    • scroll tests
    • filtercheckboxesbytext tests
    • resultscontroller tests
    • checkboxlist tests
  • Remove duplication from createcategories test code

Reporting on the issue

When refreshing the tests, there look to be several different problems. Because the randomly ordered tests are are difficult to retest, a smaller common theme is noticed where elements are left scattered on the page after the test reporter. We can check for that in each test file and log the problem, to help us keep focused on fixing relvant problems.

        afterAll(function () {
            var reporter = document.querySelector(".jasmine_html-reporter");
            if (reporter.nextSibling) {
                console.warn("Test has failed to clean up after itself.", reporter.nextSibling);
            }
        });

It’s possible to deal with the problem by automatically removing all elements after the test reporter, but that ignores what’s causing the problem.
Far better is to have each test take personal responsibility for any side-effects, which helps to keep me more aware of potential issues of the code too.

Several of the tests leave the testing sandbox on the page, which really should be removed.
Currently some of the tests add an empty sandbox, but that’s not good enough. We should update the sandbox code so that it can remove the sandboxes too.

Cleaning up sandbox code

First, I’ll add the above afterAll code to the sandbox code, which tells me that it fails to clean up after itself.
Adding the following line to the end of each test, results in the logged warning going away:

sandbox.spec.js

            document.querySelector("#sandbox").remove();

But, I don’t want to have that added to every test. It can be in an afterEach block of code instead.

sandbox.spec.js

        afterEach(function () {
            document.querySelector("#sandbox").remove();
        });

I now want to develop code that removes sandboxes, and replace that afterEach line with a more appropriate call to the sandbox.

sandbox.spec.js

        it("Removes all sandboxes", function () {
            var sandboxes;
            sandbox.init({html: ""});
            sandboxes = document.querySelectorAll("#sandbox");
            sandbox.removeAll();
            expect(sandboxes.length).toBeGreaterThan(0);
            sandboxes = document.querySelectorAll("#sandbox");
            expect(sandboxes.length).toBe(0);
        });

We are told that the removeAll method doesn’t exist, so we should add that.

sandbox.js

    return {
        init: initSandpit,
        removeAll: removeAll
    };

And we are told that the removeAll function doesn’t exist, so we should add that.

sandbox.js

    function removeAll() {
    }

And we are now told that we expect there to be 0 sandboxes instead of 1, so we can remove all sandboxes.

sandbox.js

    function removeAll() {
        document.querySelectorAll("#sandbox").forEach(function (sandpit) {
            sandpit.remove();
        });
    }

And now that we have working code to remove sandboxes, we can replace the afterEach code with that instead.

sandbox.spec.js

        afterEach(function () {
            // document.querySelector("#sandbox").remove();
            sandbox.removeAll();
        });

Tidying up the tests

Now that we have the afterAll code in each test telling us when they misbehave, I can now work through each report and fix what’s wrong with them.

Tidying up scrollmanager tests

The scrollmanager test is the first one that we get a warning about. We can run just the scrollmanager tests to find that the problem doesn’t come from other code, and inspecting the elements shows us that the sandbox is still there when it shouldn’t be. The sandbox is added before each test, so we should remove it after each one too.

scrollmanager.spec.js

        afterEach(function () {
            sandbox.removeAll();
        });

And now the scrollmanager code works without warning us about other elements after the reporter. We can move on to another set of test code now.

Tidying up searchresults tests

When we run all of the tests, the searchresults tests is the next one that shows a problem in the console.

The same process as with the scrollmanager tests shows that the sandbox is there too, so removing it after each test is the answer there too.

searchresults.spec.js

        afterEach(function () {
            sandbox.removeAll();
        });

Tidying up createcategories tests

The createcategories test is the next one that we are warned about.

It’s not a sandbox causing the problem in the createcategories test. Instead, it’s a chk0 element that’s there instead. One of the tests adds a chk0 element, so we should cleanup after that test and remove chk0.

createcategories.spec.js

        it("avoids id conflicts", function () {
            document.body.innerHTML += "<div id='chk0'></div>";
            ...
            // cleanup
            document.querySelector("#chk0").remove();
        });

Other tests leave a clearAll element. It’s the searchResults.init() code that leaves the clearAll element, so we need to remove that after each test too.

createcategories.spec.js

        afterEach(function () {
            ...
            var clearAll = document.querySelector("#clearAll");
            if (clearAll) {
                clearAll.remove();
            }
        });

There’s also a results element, which is added by the searchResults.init() code too, so that also needs to be removed after each test.

createcategories.spec.js

        afterEach(function () {
            ...
            var results = document.querySelector("#results");
            if (results) {
                results.remove();
            }
        });

Remove duplication from createcategories tests

The createcategories tests have no problems now but we need to clean up the test code, as there’s a lot of duplication in the afterEach code now.

createcategories.spec.js

        afterEach(function () {
            search = document.querySelector("#categorysearch");
            if (search) {
                search.remove();
            }
            categories = document.querySelector("#categories");
            if (categories) {
                categories.remove();
            }
            var clearAll = document.querySelector("#clearAll");
            if (clearAll) {
                clearAll.remove();
            }
            var results = document.querySelector("#results");
            if (results) {
                results.remove();
            }
        });

Instead of all that duplicated code, we can have a small helper function to test for and remove any given selector.

createcategories.spec.js

        afterEach(function () {
            function removeBySelector(selector) {
                document.querySelectorAll(selector).forEach(function (el) {
                    el.remove();
                });
            }
            removeBySelector("#categorysearch");
            removeBySelector("#categories");
            removeBySelector("#clearAll");
            removeBySelector("#results");
        });

We could even put those selectors in an array and use forEach to call removeBySelector, but in this situation that wouldn’t help to make the code easier to understand, so the above code works well as it is.

The above improvement is a lot tidier, and is a good example of the benefits that are gained by waiting until we have at least three examples of duplication before removing that duplication.

Fixing scroll tests

The scroll code is the next set of tests that report a problem, which is a simple sandbox to fix up in the same way as with the others.

scroll.spec.js

        afterEach(function () {
            sandbox.removeAll();
        });

Fixing filtercheckboxesbytext tests

The sandbox is the same issue here, which is easily fixed too.

filtercheckboxesbytext.spec.js

        afterEach(function () {
            sandbox.removeAll();
        });

Fixing resultscontroller test code

The sandbox is the same issue here, which is easily fixed too.

resultscontroller.spec.js

        afterEach(function () {
            sandbox.removeAll();
        });

Fixing checkboxlist test code

The sandbox is the same issue here, which is easily fixed too.

checkboxlist.spec.js

        afterEach(function () {
            sandbox.removeAll();
        });

And all tests now work with no problems reported to the console.


An important lesson here is to clean up after yourself. I’m going to leave the afterAll code in the tests, because it’s a handy automatic way to help me catch other situations too.

With a full set of working tests, I now feel better about moving forward with the works in a variety of common situations tests, and then replace existing HTML code new code generated from the categories.json data file.


#46

Now that the tests are all behaving properly after having made createcategories and createresults, I can finish off the createcategories tests and make sure that the searchresults init code works properly in a wide variety of situations.

What we do today is:

  • Add tests for a variety of searchresults situations
  • fix code coverage across all files
  • fix createcategories output

Tests for a variety of searchresults situations

First, as these tests are tightly related to each other, we should create a separate place for these tests.

createcategories.spec.js

        describe("init works in a variety of common situations", function () {
            it("with categories in HTML and no category data file", function () {
            });
            it("with categories in HTML replaced with category data file", function () {
            });
            it("no categories and category data file", function () {
            });
        });

In the existing situation with no data file, we want to make sure that the existing categories element is not affected. If the element is removed from the page it will have no parentNode attribute, so we can expect that it should not be null after running the init code.

createcategories.spec.js

            it("with categories in HTML and no category data file", function () {
                categories = document.querySelector("#categories");
                expect(categories).toBeNull();
                document.body.innerHTML += "<div id='categories'></div>";
                categories = document.querySelector("#categories");
                expect(categories.parentNode).not.toBeNull();
                searchResults.init();
                expect(categories.parentNode).not.toBeNull();
                // cleanup
                categories.remove();
            });

When there is a data file and the categories are already shown on the page, we want to remove those existing categories and replace them with a new set generated from that data file.

We can expect that when the old set of categories is removed, that it will no longer have any parents.

createcategories.spec.js

            it("with categories in HTML replaced with category data file", function () {
                categories = document.querySelector("#categories");
                expect(categories.parentNode).toBeNull();
                document.body.innerHTML += "<div id='categories'></div>";
                categories = document.querySelector("#categories");
                expect(categories.parentNode).not.toBeNull();
                searchResults.init(catData);
                expect(categories.parentNode).toBeNull();
                categories = document.querySelector("#categories");
                expect(categories).not.toBeNull();
                // Cleanup
                categories.remove();
            });
        });

When there’s a data file and no categories on the page, it needs to create a place for those categories and fill it with a new set generated from the data file.

createcategories.spec.js

            it("no categories and category data file", function () {
                categories = document.querySelector("#categories");
                expect(categories).toBeNull();
                searchResults.init(catData);
                categories = document.querySelector("#categories");
                expect(categories).not.toBeNull();
            });
        });

Those are the three different types of situations that the code needs to succesfully deal with.

Eash test does the same cleaning up afterwards, which can be moved out of each test and in to a separate afterEach method.

createcategories.spec.js

        describe("init works in a variety of common situations", function () {
            afterEach(function () {
                categories.remove();
            });
            it("with categories in HTML and no category data file", function () {
                ...
                // Cleanup
                // categories.remove();
            });
            it("with categories in HTML replaced with category data file", function () {
                ...
                // Cleanup
                // categories.remove();
            });
            it("no categories and category data file", function () {
                ...
                // Cleanup
                // categories.remove();
            });
        });

And because just having an afterEach method by itself can be somewhat confusing, we should also have a beforeEach method that sets up the categories that will eventually be removed.

createcategories.spec.js

        describe("init works in a variety of common situations", function () {
            beforeEach(function () {
                categories = document.querySelector("#categories");
            });
            afterEach(function () {
                categories.remove();
            });
            it("with categories in HTML and no category data file", function () {
                // categories = document.querySelector("#categories");
                expect(categories).toBeNull();
                ...
            });
            it("with categories in HTML replaced with category data file", function () {
                // categories = document.querySelector("#categories");
                expect(categories).toBeNull();
                ...
            });
            it("no categories and category data file", function () {
                // categories = document.querySelector("#categories");
                expect(categories).toBeNull();
                ...
            });
        });

Code coverage

Now that the tests pass, I should go through code coverage reports to ensure that all of the code is being tested.
Some of our code files don’t have enough code coverage. Those files are:

  • createcategories.js
  • createresults.js

Createcategories coverage issues

The first issue is with the following code:

createcategories.js

        if (!document.getElementById(id)) {
            console.log("return id;");
            return id;
        } // no code coverage on this line
        return makeId(prefix);

The code coverage failure is seems to be complaining about having multiple return paths. Returning from a guard clause is not seen as being a good thing.
We can fix that issue here by using an else clause, which helps to provide a clearer reason for the multiple returns.

createcategories.js

        if (!document.getElementById(id)) {
            return id;
        // }
        } else {
            return makeId(prefix);
        }

A similar issue occurs further down in the code too:

createcategories.js

        if (!catData || !search) {
            return;
        } // no code coverage on this line
        renewCategories(categories, search);
        categories = document.querySelector("#categories");
        categories.appendChild(addCategoryItems(catData));
        return categories;

We can invert the if statement here by turning !a || !b into a && b instead, and place the code that we want to execute inside of the if statement.

createcategories.js

        if (catData && search) {
            renewCategories(categories, search);
            categories = document.querySelector("#categories");
            categories.appendChild(addCategoryItems(catData));
        }
        return categories;

The coverage is happy with this code now.

Createresults code coverage issue

The other coverage issue is with similar code in createresults.js

createresults.js

        if (!catData || !categories) {
            return;
        }
        clearAll = renewClearAll(clearAll, categories);
        results = renewResults(results, clearAll);

How we fix this is the same as with the other above code. We invert the if condition and place the code we want to execute inside the if statement instead.

createresults.js

        if (catData && categories) {
            clearAll = renewClearAll(clearAll, categories);
            results = renewResults(results, clearAll);
        }

Now that the tests and the coverage are all good, I can move on with fixing the createcategories code.

Fixing the createcategories output

Checking the visual display of the categories shows that something is out of alignment. Currently they are being added below the #categorysearch element, when they need to be added below the #search form instead.

It’s time to fix things, and use #search in the test instead of using #categorysearch.

createcategories.spec.js

        beforeEach(function () {
            // document.body.innerHTML += "<input id='categorysearch' name=`createcategoriestest`>";
            document.body.innerHTML += "<form id='search'></form>";
            // search = document.querySelector("#categorysearch");
            search = document.querySelector("#search");
        });
        ...
            // removeBySelector("#categorysearch");
            removeBySelector("#search");

Those createcategories tests fail, which can now be made to pass by using searchForm instead of searchField

searchresults.js

                // var search = document.querySelector(searchField);
                var search = document.querySelector(searchForm);

The createresults tests are the only non-working tests to deal with now.

createresults.spec.js

        beforeEach(function () {
            // document.body.innerHTML += "<input id='categorysearch' name=`createresultstest`>";
            document.body.innerHTML += "<form id='search'></form>";
        });
        ...
            // removeBySelector("#categorysearch");
            removeBySelector("#search");

And all of the tests pass once more.


Everything with createcategories is now good and working well, and we only have the createresults code left to go, which I’ll deal with in the next update.


#47

I was going to ignore some issues with createcategories until after I’d done the createresults code, but I ended up having to deal with the createcategories issues anyway, so first we’ll get those issues out of the way.

This has helped to realize an important aspect of development, to ensure that everything is still working before moving on to the next thing.

createcategories issue

The createcategories code has resulted in the category clicks not showing results, and the tests aren’t showing what’s the problem because searchresults is mostly initialized without the JSON file.

Instead of doing a whole separate test files with that, I’ll instead figure out which tests are affected by this, and add a few regression tests to help catch this situation and prevent further occurrances of it.

The first issue is that “#chkFruits” is used by some of the tests as a selector, even though the HTML code generated from the JSON file results in a different identifier of “#chk0”. We can instead just get the first input field and that will work regardless of the id attribute.

searchresults.spec.js

        it("shows a result when a category is selected", function () {
            // var item = categories.querySelector("#chkFruits");
            var item = categories.querySelector("input");
            expect(hasVisibleResults()).toBe(false);
            item.click();
            expect(hasVisibleResults()).toBe(true);
        });

The test still fails though with the item being “#chkFruits”, even though no such thing exists on the page. What’s going on there?

The beforeEach section of code gets the categories before running the init code.

searchresults.spec.js

            categories = sandpit.querySelector("#categories");
            clearAllButton = sandpit.querySelector("#clearAll");
            searchResults.init();

After the init code has run, the categories variable will still be pointing to the old set of categories, not the new lot that have been added to the page. We need to move that init code to the top of the section of code for things to work.

createcategories.spec.js

            searchResults.init();
            categories = sandpit.querySelector("#categories");
            clearAllButton = sandpit.querySelector("#clearAll");
            // searchResults.init();

And now the correct item is retrieved, but it still doesn’t show the result. Why?

The search filter test expects for there to be 2 categories (from a simplified category test) but the JSON data ends up showing 4 categories instead.

createcategories.spec.js

        it("uses a search filter", function () {
            var categorysearch = $("#categorysearch");
            // expect($("#categories p:visible").length).toBe(2);
            expect($("#categories p:visible").length).toBe(4);

That’s about the only extra test that’s required with regard to the JSON file. The tests all now pass, and we only have some cleaning up to do, because “Categories to toggle” is being left on the test page.

Categories to toggle

The “Categories to toggle” message is not being removed from the tests, and while it’s possible to find and remove the appropriate paragraph, I think that a better solution might be to contain the search results inside of its own div, making it easier to clean up afterwards by emptying the div.

But first, we need to understand why that message is remaining on the screen.

createcategories test is responsible

I think that the main problem is in how I’m removing the test code.

createcategories.spec.js

        afterEach(function () {
            function removeBySelector(selector) {
                document.querySelectorAll(selector).forEach(function (el) {
                    el.remove();
                });
            }
            removeBySelector("#search");
            removeBySelector("#categories");
            removeBySelector("#clearAll");
            removeBySelector("#results");
        });

Instead of doing all of those removals, I need instead to just put them in a sandbox, letting me easily remove the sandbox afterwards.

createcategories.spec.js

define([
    // "json!categories", "createcategories", "searchresults"
    "../tests/sandbox", "json!categories", "createcategories", "searchresults"
// ], function (createCategories, searchResults) {
], function (catData, createCategories, searchResults) {
...
        beforeEach(function () {
            // document.body.innerHTML += "<form id='search'></form>";
            sandpit = sandbox.init({
                html: "<form id='search'></form>",
                visible: false
            });
            // search = document.querySelector("#search");
            search = sandpit.querySelector("#search");
        });
        afterEach(function () {
            sandbox.removeAll();
            // function removeBySelector(selector) {
            //     document.querySelectorAll(selector).forEach(function (el) {
            //         el.remove();
            //     });
            // }
            // removeBySelector("#search");
            // removeBySelector("#categories");
            // removeBySelector("#clearAll");
            // removeBySelector("#results");
        });

That fixes the main problem, leaving only a few other issues to tidy up by putting them in the sandpit too.

createcategories.spec.js

        it("removes pre-existing categories sections", function () {
            // document.body.innerHTML += "<div id='categories'></div>";
            sandpit.innerHTML += "<div id='categories'></div>";
            // categories = document.querySelector("#categories");
            categories = sandpit.querySelector("#categories");
            ...
        });
        ...
            it("with categories in HTML and no category data file", function () {
                expect(categories).toBeNull();
                // document.body.innerHTML += "<div id='categories'></div>";
                document.body.innerHTML += "<div id='categories'></div>";
                // categories = document.querySelector("#categories");
                categories = sandpit.querySelector("#categories");
                ...
            });
            it("with categories in HTML replaced with category data file", function () {
                categories = document.querySelector("#categories");
                expect(categories).toBeNull();
                // document.body.innerHTML += "<div id='categories'></div>";
                sandpit.innerHTML += "<div id='categories'></div>";
                // categories = document.querySelector("#categories");
                categories = sandpit.querySelector("#categories");
                ...
            });

Ensuring that we use the sandpit for our tests helps to ensure that it’s easier for the tests to clean up afterwards.

Now that the tests and the code are back to being fully working, the create results code is what we’ll work on next.


#48

Results format

Before reproducing the HTML code, I want to clean it up first. Currently the results seciton of HTML code looks like this:

<p>Results: <button id="clearAll" style="display: none">Clear all results</button></p>
<div id="results">
  <div class="mydivs1" id="result0">
    <div class="mydivs2">
      <button class="close">X</button>
    </div>
    <div class="mydivs3">
      <span class="myspan1">Fruits</span>
      <p>
        <span class="myspan2">1:</span>
        <br>
        <span class="myspan3">Apples</span>
        <p></p>
        <span class="myspan2">2:</span>
        <br />
        <span class="myspan3">Blackberries</span>
        <p></p>
        <span class="myspan2">3:</span>
        <BR />
        <span class="myspan3">Raspberries</span>
    </div>
    <p></p>
  </div>

  <p></p>
  ...

The inline style must go, along with all of the breaks, and the empty paragraphs. The generic class names like mydivs1 are really bad too.
We can remove all of those and replace the results with an ordered list, using styling to make it look the same as what you had with the other HTML code.

style.css

#clearAll {
    display: none;
}
.result ol {
  padding: 0;
  list-style-position: inside;
}
.result ol > li {
  margin-top: 1em;
}
.result ol > li:before {
  content: "";
  display: block;
  margin-bottom: 0.3em;
}

searchresults.js

        // var resultSelector = ".mydivs1";
        var resultSelector = ".result";

index.html

  <div class="result" id="result0">
    <div class="sidebar">
      <button class="close">X</button>
    </div>
    <div class="content">
      <p>Fruits</p>
      <ol>
        <li>Apples</li>
        <li>Blackberries</li>
        <li>Raspberries</li>
      </ol>
    </div>
  </div>

Replacing the index.html file code with the above, everything carries on working well and we have a good base from which to build.

The createresults code is very similar to the createcategories code, resulting in the following tests and code.

createresults.js

/*jslint browser */
/*global define */
define([], function () {
    "use strict";
    function renewClearAll(clearAll, categories) {
        if (clearAll) {
            clearAll.remove();
        }
        clearAll = document.createElement("p");
        clearAll.id = "clearAll";
        clearAll.innerHTML = "Results: <button class='clearAll'>Clear all results</button>";
        categories.insertAdjacentElement("afterend", clearAll);
        return clearAll;
    }
    function renewResults(results, catData, clearAll) {
        // If the results section exists, it needs to be removed and recreated.
        // Just emptying the results section is not good enough, as that
        // can leave things like unwanted event handlers behind.
        if (results) {
            results.remove();
        }
        results = document.createElement("div");
        results.id = "results";
        clearAll.insertAdjacentElement("afterend", results);

        results.innerHTML = `
  <div class="result" id="result0">
    <div class="sidebar">
      <button class="close">X</button>
    </div>
    <div class="content">
      <p>Fruits</p>
      <ol>
        <li>Apples</li>
        <li>Blackberries</li>
        <li>Raspberries</li>
      </ol>
    </div>
  </div>
  <div class="result" id="result1">
    <div class="sidebar">
      <button class="close">X</button>
    </div>
    <div class="content">
      <p>Vegetables</p>
      <ol>
        <li>Beets</li>
        <li>Eggplants</li>
        <li>Spinach</li>
      </ol>
    </div>
  </div>
  <div class="result" id="result2">
    <div class="sidebar">
      <button class="close">X</button>
    </div>
    <div class="content">
      <p>Nuts</p>
      <ol>
        <li>Almonds</li>
        <li>Pecans</li>
        <li>Walnuts</li>
      </ol>
    </div>
  </div>
  <div class="result" id="result3">
    <div class="sidebar">
      <button class="close">X</button>
    </div>
    <div class="content">
      <p>Meats</p>
      <ol>
        <li>Chicken</li>
        <li>Fish</li>
        <li>Turkey</li>
      </ol>
    </div>
  </div>
`;
        return results;
    }
    function init(catData, categories) {
        var clearAll = document.querySelector("#clearAll");
        var results = document.querySelector("#results");
        if (catData && categories) {
            clearAll = renewClearAll(clearAll, categories);
            renewResults(results, catData, clearAll);
        }
    }
    return {
        init: init
    };
});

The categories and the results HTML code can now be removed from the index.html file, and we can work on having the createresults code actually generate the html code that’s required.

Generating results

Currently it’s the same set of results being added to the page. Instead of that, we want break things up into each result, so that we can eventually generate them fully from the data file.

What now follows is a process of simplifying, and reducing the html content to information stored in an array.

createresults.js

        var arr = [
          "<div class='result' id='result0'>" +
            "<div class='sidebar'>" +
              "<button class='close'>X</button>" +
            "</div>" +
            "<div class='content'>" +
              "<p>Fruits</p>" +
              "<ol>" +
                "<li>Apples</li>" +
                "<li>Blackberries</li>" +
                "<li>Raspberries</li>" +
              "</ol>" +
            "</div>" +
          "</div>",
          ...
        ];
        results.innerHTML = arr.reduce(function (html, item) {
            return html + item;
        }, "");

I’ve used reduce here instead of just join, because I need the index value for the next stage, where the code around the items is generated.

createresults.js

        var arr = [
            "<p>Fruits</p>" +
            "<ol>" +
                "<li>Apples</li>" +
                "<li>Blackberries</li>" +
                "<li>Raspberries</li>" +
            "</ol>",
            ...
        ];
        results.innerHTML = arr.reduce(function (html, item, index) {
            return html +
                "<div class='result' id='result" + index + "'>" +
                "  <div class='sidebar'>" +
                "    <button class='close'>X</button>" +
                "  </div>" +
                "  <div class='content'>" +
                item +
                "  </div>" +
                "</div>";
        }, "");

We can now use the catData file to create the array of category information:

createresults.js

        function resultList(catInfo) {
            var items = catInfo.items.map(function (item) {
                return "<li>" + item + "</li>";
            });
            return "<p>" + catInfo.title + "</p>" +
                "<ol>" + items + "</ol>";
        }
        function resultReducer(html, item, index) {
            return html +
                "<div class='result' id='result" + index + "'>" +
                "  <div class='sidebar'>" +
                "    <button class='close'>X</button>" +
                "  </div>" +
                "  <div class='content'>" +
                item +
                "  </div>" +
                "</div>";
        }
        results.innerHTML = catData.map(resultList).reduce(resultReducer, "");

The code is now creating categories and results entirely based on the JSON categories file.

Next step

Though the code is now creating custom results from the JSON categories file, I’m not happy about how HTML code is intermixed with the JavaScript code.
A better solution is to use some small templating code called Mustache, so that we can put the template in the HTML code, something like this:

<script id="template" type="x-tmpl-mustache">
{{#results}}
  <div class="result" id="{{resultid}}">
    <div class="sidebar">
      <button class="close">X</button>
    </div>
    <div class="content">
      <p>{{title}}</p>
      {{#items}}
      <ol>
        {{#food}}
        <li>.</li>
        {{/foods}}
      </ol>
      {{/items}}
    </div>
  </div>
{{/results}}
</script>

I’ll take a look next about how templates can help us to have much simpler code.


#49

The mustache code is obtained from https://github.com/janl/mustache.js/releases

I’ve placed the mustache.min.js file in the lib folder and won’t be using the npm install or build parts of the process. Instead, the template library is just being used to convert index.html code into appropriate categories and results.

How do we get the Mustache template working? We need to able to tell if it is working, and so a test is what we write to tell if something is working. When the test passes we can then know that it is working.

But how do we tell that it’s working? We shouldn’t access Mustache directly from the test, and should instead look for changes that the correctly working Mustache will achieve.

But how do we know what the test is supposed to achieve when we don’t really know how Mustache works?

This is where a Spike comes in handy.

Spike

A Spike helps us to wedge upen a crack, and is where we temporarily ignore tests and just drive the code through all abstraction layers. We can then code up some tests based on what we’ve learned from that spike, then remove it.

In this case, we can place a simple use of Mustache in index.html, to find out how to use it.

The template itself we can put just below the existing form:

index.html

<script id="result-template" type="x-tmpl-mustache">
{{#results}}
  <div class="result" id="{{resultid}}">
    <div class="content">
      <p>{{title}}</p>
      <ol>
      {{#food}}
        <li>{{.}}</li>
      {{/food}}
      </ol>
    </div>
  </div>
{{/results}}
</script>

The script tag easily hides the content from normal view, and makes it easy for the template to be obtained.

The Mustache library is loaded before the requirejs code. We will eventually remove that script when we have requirejs handle that instead.

<script src="js/lib/mustache.min.js"></script>
<script type="text/javascript" src="js/lib/require/require.js" data-main="js/src/main"></script>

And the script itself gets the template and renders it with suitable view information.

index.html

<script>
var template = document.querySelector("#result-template");
var target = template.insertAdjacentHTML("afterend", document.createDocumentFragment());
var view = {
    results: {
        resultid: "result0",
        title: "Fruits",
        food: ["Apples", "Blackberries", "Raspberries"]
    }
};
Mustache.parse(template.innerHTML);
var rendered = Mustache.render(template.innerHTML, view);
target.append(rendered);
</script>

Updating the code

Now that we have the Mustache template working and understand how it’s used, we can move on to updating the code. But what about writing a test. Do we have to?

Our tests shouldn’t care about how something is done, only that it is done. We already have the tests to ensure that things are working right.

Because we already have all the tests that we need, we can instead delete most of the createcategories code and replace it with the Mustache template code instead, which will happen in the next post.


#50

Now that we have a spike working for the Mustache template, we can replace some parts of the code with the Mustache template equivalent.

Template from existing code

The resultReducer function looks to be the best place to update:

createcategories.js

    function resultReducer(html, catInfo, index) {
        return html +
            "<div class='result' id='result" + index + "'>" +
            "  <div class='sidebar'>" +
            "    <button class='close'>X</button>" +
            "  </div>" +
            "  <div class='content'>" +
            resultList(catInfo) +
            "  </div>" +
            "</div>";
    }

I want to do this update so that we can easily get back to a working system if things go wrong.

Here’s the template we want, which looks like the output from the above code:

index.html

<script id="result-template" type="x-tmpl-mustache">
{{#results}}
  <div class="result" id="result{{id}}">
    <div class='sidebar'>
      <button class='close'>X</button>
    </div>
    <div class="content">
      <p>{{title}}</p>
      <ol>
      {{#food}}
        <li>{{name}}</li>
      {{/food}}
      </ol>
    </div>
  </div>
{{/results}}
</script>

We can replace the spike template with that above template, just below the clearAll code in the index file.

Protect working code until replacement works

We will need the mustache library, so we include that at the top of the createcategories code:

createcategories.js

// define([], function () {
define(["mustache"], function (mustache) {

We can now use the scripting code from the spike, to connect things together.

createcategories.js

    function resultReducer(html, catInfo, index) {
        // return html +
        //     "<div class='result' id='result" + index + "'>" +
        //     "  <div class='sidebar'>" +
        //     "    <button class='close'>X</button>" +
        //     "  </div>" +
        //     "  <div class='content'>" +
        //     resultList(catInfo) +
        //     "  </div>" +
        //     "</div>";
        var template = document.querySelector("#result-template");
        var foodItems = catInfo.items.map((item) => item.name + item.code);
        var view = {
            results: {
                id: index,
                title: catInfo.title,
                food: catInfo.items
            }
        };
        mustache.parse(template.innerHTML);
        return html + mustache.render(resultTemplate.innerHTML, view);
    }

I’ve left the previous code as being commented out, so that we can easily go back to it if needed.

Using the Food codes

Those food items also have a code associated with each of them, which we can use in any way desires, such as with:

index.html

        <li data-code="{{code}}">{{name}}</li>

That way the product code is accessible to anything that needs it, but it isn’t shown on the screen.

Get the tests working

That’s all working well but the test isn’t happy. The requested template isn’t available in the test code. We can add a primitive result section to keep the test happy:

createresults.spec.js

        beforeEach(function () {
            sandpit = sandbox.init({
                html: "<form id='search'></form>" +
                "<script id='result-template' type='x-tmpl-mustache'>" +
                "{{#results}}" +
                "<ol>" +
                "  {{#food}}" +
                "  <li>{{name}}</li>" +
                "{{/food}}" +
                "</ol>" +
                "{{/results}}" +
                "</script>"
            });
        });

And, we can have it only do the template stuff if the template exists.

    "use strict";
    var resultTemplate;
    ...
    function init(catData, categories) {
        ...
        resultTemplate = document.querySelector("#result-template");
        // if (catData && categories) {
        if (resultTemplate) {
            ...
        }
    }

And all of the tests pass.

Remove the spike and clean up

We can now remove the Spike code from before:

index.html

<!--
<script src="js/lib/mustache.min.js"></script>
-->
<script type="text/javascript" src="js/lib/require/require.js" data-main="js/src/main"></script>
<!--
<script>
var template = document.querySelector("#result-template");
var view = {
    results: {
        resultid: "result0",
        title: "Fruits",
        food: ["Apples", "Blackberries", "Raspberries"]
    }
};
Mustache.parse(template.innerHTML);
var rendered = Mustache.render(template.innerHTML, view);
template.insertAdjacentHTML("afterend", rendered);
</script>
-->

Other cleaning up is to remove the resultList function, and remove the old commented-out code from resultReducer:

createresults.js

    // function resultList(catInfo) {
    //     var items = catInfo.items.map(function (item) {
    //         return "<li>" + item.name + "</li>";
    //     });
    //     return "<p>" + catInfo.title + "</p>" +
    //         "<ol>" + items.join("") + "</ol>";
    // }

and we’re left with a working Mustache template, where the index page contains the template in the correct place, and the scripting code just gets that template and gives it the view-info that’s needed.

Converting the categories code to use a similar template is what we’ll do next.


#51

Converting the categories code to use a template is up next.

Accessing the template

We request the mustache library at the top of the code:

createcategories.js

define(["mustache"], function (mustache) {

Here’s the template that we’ll place above the results section:

index.html

<p>Categories to toggle</p>
<div id="categories">
<script id="category-template" type="x-tmpl-mustache">
{{#category}}
  <p><input type="checkbox" id="chk{{index}}" value="result{{index}}">
    <label for="chk{{index}}">{{title}}</label></p>
{{/category}}
</script>
</div>

Then we get that template section and store a reference to it, so that other code can access it:

createcategories.js

    var categoryTemplate;
    ...
    function init(catData, search) {
        categoryTemplate = document.querySelector("#category-template");
        ...
    }

Use the template to generate content

We can then replace the addCategoryItems code to render from the template instead.

createcategories.js

    function categoryReducer(html, catInfo, index) {
        var view = {
            category: {
                index: index,
                title: catInfo.title
            }
        };
        // mustache.parse(categoryTemplate.innerHTML);
        return html + mustache.render(categoryTemplate.innerHTML, view);
    }
    ...
        if (categoryTemplate) {
            categories = renewCategories(categories, search);
            // categories.appendChild(addCategoryItems(catData));
            categories.innerHTML = catData.reduce(categoryReducer, "");
        }

The mustache.parse is removed from the above code, for on further reading about it, it isn’t needed. It’s useful if you frequently re-render the same template, but there’s no benefit when you’re just rendering things once.

This has broken some tests though which must be fixed, which will be the focus of the next post.


#52

Fixing the tests

This is always the uncomfortable part. The above changes has broken some tests, and we need to get back to fully working tests.

The dilemma here is that, for each test that needs to be fixed, do we:

  • change the test?
  • change the code?
  • remove the test?
  • change our approach entirely?

Previously three different scenarios were handled with the categories and results.

  • HTML categories and results, without category JSON file
  • HTML categories and results, with category JSON file
  • No HTML categories and results, with category JSON file

Now that the template is in the HTML code, only the last situation remains valid. There’s no point retaining tests for situations that can no longer occur, so we can remove tests relating to that.

  • createcategories removes pre-existing categories sections
  • createcategories doesn’t error when categories doesn’t exist
  • createcategories init works under common situations
  • createcategories with HTML categories and no category data file
  • createcategories with replaced HTML and a category data file
  • createcategories with no categories and category data file

Fixing remaining tests

Adding a template to the sandbox fixes most of the remaining issues that the tests are having.

createcategories.spec.js

            sandpit = sandbox.init({
                html: "<form id='search'></form>" +
                    "<p>Categories to toggle</p>" +
                    "<div id='categories'>" +
                    "<script id='category-template' type='x-tmpl-mustache'>" +
                    "{{#category}}" +
                    "  <p><input type='checkbox' id='chk{{index}}' value='result{{index}}'>" +
                    "    <label for='chk{{index}}'>{{title}}</label></p>" +
                    "{{/category}}" +
                    "</script>" +
                    "</div>",

Search results needs to have the templates there too for the code to work, which is not all that desirable.

searchresults.spec.js

    var sandpitHtml = "<form id='search'>" +
            "<p><input type='text' id='categorysearch' " +
            "    name='searchresultstest'></p>" +
            "</form>" +
            "<p>Categories to toggle</p>" +
            "<div id='categories'>" +
            "<script id='category-template' type='x-tmpl-mustache'>" +
            "{{#category}}" +
            "  <p><input type='checkbox' id='chk{{index}}' " +
            "    value='result{{index}}'>" +
            "    <label for='chk{{index}}'>{{title}}</label></p>" +
            "{{/category}}" +
            "</script>" +
            "</div>" +
            "<p id='clearAll'>Results:" +
            "  <button class='clearAll'>Clear all results</button>" +
            "</p>" +
            "<script id='result-template' type='x-tmpl-mustache'>" +
            "{{#results}}" +
            "  <div class='result' id='result{{index}}'>" +
            "    <div class='sidebar'>" +
            "      <button class='close'>X</button>" +
            "    </div>" +
            "    <div class='content'>" +
            "      <p>{{title}}</p>" +
            "      <ol>" +
            "      {{#food}}" +
            "        <li data-code='{{code}}'>{{name}}</li>" +
            "      {{/food}}" +
            "      </ol>" +
            "    </div>" +
            "  </div>" +
            "{{/results}}" +
            "</script>";

Wow, that’s a lot of template to add, where the categories and results templates are combined, but everything is now working. I’d rather not need to do that.

Next time

Some problems can now be worked on and improved. For example, I’m not happy that the searchresults tests require mustache templates when it’s not supposed to know about such things.

Next time I’ll work on moving those tests around, so that category and result tests are removed from searchresults, and we shouldn’t then need as much setup in those tests.


#53

I don’t want all of the template code in the createresults tests area. I want to move the category search stuff in the category test, and the results stuff in the results test.

Looking at the searchresults file, there are clearly functions that belongs in the checkboxlist or resultscontroller instead.

  • updateResults
  • clearAllClickHandler
  • uncheckCheckbox

We can see to the updateResults code today.

Move updateResults to resultscontroller

The update function can be moved into the resultscontroller code, where we make a few adjustments so that it accesses local functions:

resultscontroller.js

    var clearAllButton = "#clearAll";
    ...
    function update(checkbox) {
        var result = $("#" + checkbox.value);
        // resultsController.toggleResult(result);
        toggleResult(result);
        // var showClearAll = resultsController.hasVisible();
        var showClearAll = hasVisible();
        $(clearAllButton).toggle(showClearAll);
    }
    return {
        ...
        update: update,
        ...
    };

The toggleResult function doesn’t need to exist anymore, and the hasVisible function is only used by the test code now, so we should remove that too.

Inline toggletResult function

We can get rid of the toggleResult function, inlining the toggleResult function into the update function instead.

resultscontroller.js

    function update(checkbox) {
        ...
        // toggleResult(result);
        $(result).toggle();
        ...
    }

searchresults.js

        // function updateResults(checkbox) {
        //     var result = $("#" + checkbox.value);
        //     resultsController.toggleResult(result);
        //     var hasVisible = resultsController.hasVisible();
        //     $(clearAllButton).toggle(hasVisible);
        // }
        ...
            checkboxList.init({
                container: $(categories),
                // afterClick: updateResults
                afterClick: resultsController.update
            });

Restructure to remove toggleResults function

I also want to remove the toggleResult function, but that causes some tests to break that cannot yet be easily updated without breaching a separation of concern. We should instead have the checkboxlist code supply that info. We can then have searchresults manage that transfer of information instead.

While I could do this in a faster way, that leaves the code in a non-working state throughout the process, and I want to try and ensure that everything remains working at every stage. Not only does that help you to think more clearly about the code, but if the code needs to be used, you’re able to rapidly get it going.

Move getValue code into checkboxlist

The update function in resultscontroller though shouldn’t need to know about the checkbox, so we should have checkboxlist get us the value instead.

checkboxlist.spec.js

        it("gives the value of the checkbox", function () {
            var checkbox = items[0];
            var id = checkboxList.getValue(checkbox);
            expect(id).toBe("One");
        });

checkboxlist.js

    function getValue(checkbox) {
        return checkbox.value;
    }
    return {
        getValue: getValue,
        ...
    };

We can now have the update function use the value from that getValue function instead:

resultscontroller.js

    // function update(checkbox) {
    function update(id) {
        // var result = $("#" + checkbox.value);
        var result = $("#" + id);
        ...
    }

searchresults.js

            checkboxList.init({
                ...
                afterClick: function (checkbox) {
                    var id = checkboxList.getValue(checkbox);
                    return resultsController.update(id);
                }
            });

That is a good and proper situation for that section of code in searchresults. All it does is to get info from checkboxlist, and give it to resultscontroller.

Fixing test that show/hide a result

We can now remove toggleResult from the resultscontroller code:

resultscontroller

    // function toggleResult(result) {
    //     $(result).toggle();
    // }
    ...
    return {
        ...
        // toggleResult: toggleResult,
        ...
    };

which causes two tests to break, and we can now easily modify to make them pass. Instead of testing toggleResult, they can achieve the test via the update function instead.

            it("can show a result", function () {
                // var item = sandpit.querySelector("#Fruits");
                var id = "Fruits";
                expect(resultsController.hasVisible()).toBe(false);
                // resultsController.toggleResult(item);
                resultsController.update(id);
                expect(resultsController.hasVisible()).toBe(true);
            });
            it("can hide a result", function () {
                // var item = sandpit.querySelector("#Fruits");
                var id = "Fruits";
                // $(item).show();
                $("#" + id).show();
                expect(resultsController.hasVisible()).toBe(true);
                // resultsController.toggleResult(item);
                resultsController.update(id);
                expect(resultsController.hasVisible()).toBe(false);
            });

With the updateResults function removed and replaced with a more appropriate update function, the tests all pass.

Next steps

Commenting out the afterClick line shows that the clearAll tests all rely on it, so the clearAll code and tests should all move across to the resultscontroller section too, which we’ll work on next.

After that, we’ll move the uncheckCheckbox and clearAllClickHandler functions to a better place, which should allow us to more easily achieve the main objective, of removing duplicate template code from the searchresults tests.


#54

Moving the clearAll code and tests across to the resultscontroller is up next. Here’s what we’ll be doing:

  • move clearAll tests to resultscontroller
  • rename update to toggle
  • improve the clearAll handler

Move clearAll tests to resultscontroller

First, the tests are grouped together at the bottom of the searchresults tests.

searchresults.spec.js

        describe("clearAll button", function () {
            var clearAllButton;
            beforeEach(function () {
                clearAllButton = sandpit.querySelector("#clearAll");
            });
            it("shows the clearAll button when a result is shown", function () {
                ...
            });
            it("hide clearAll when no more results are shown", function () {
                ...
            });
            it("clearAll clears all of the results", function () {
                ...
            });
        });

The resultscontroller test needs a clearAll section:

resultscontroller.spec.js

                // html: "<div id='results'>" +
                html: "<p id='clearAll'>Results:" +
                    "  <button class='clearAll'>Clear all results</button>" +
                    "</p>" +
                    "<div id='results'>" +

and the earlier grouping of clearAll tests helps is to more easily move them over with the resultscontroller tests.

resultscontroller.spec.js

        it("shows the clearAll button when results shown", function () {
            var id = "Fruits";
            resultsController.update(id);
            expect(isVisible(clearAllButton)).toBe(true);
            resultsController.update(id);
            expect(isVisible(clearAllButton)).toBe(false);
        });

And with that, the clearAll tests in the searchresults tests can be removed.

    // function isVisible(obj) {
    //     return obj.offsetWidth > 0 && obj.offsetHeight > 0;
    // }
    // describe("clearAll button", function () {
    //     ...
    //     it("shows the clearAll button when a result is shown", function () {
    //         ...
    //     });
    //     it("hide clearAll when no more results are shown", function () {
    //         ...
    //     });
    //     it("clearAll clears all of the results", function () {
    //         ...
    //     });
    // });

Renaming update to toggle

While doing the above test, it becomes clear that update function is much better named to toggle, so renaming things eally should be done now.

resultscontroller.js

    // function update(id) {
    function toggle(id) {
    ...
    return {
        ...
        // update: update,
        toggle: toggle,

searchresults.js

                    // return resultsController.update(id);
                    return resultsController.toggle(id);

resultscontroller.spec.js

                // resultsController.update(id);
                resultsController.toggle(id);

Improving the clearAll handler

Currently the searchresults code fiddles around with the checkboxes.

searchresults.js

        function clearAllClickHandler() {
            resetTextField(searchField);
            $(":checked", categories).click(); // problem
        }
``

That code is better placed in the checkboxes area.

So first as usual, we test that visible items are turned off:

checkboxlist.spec.js
```javascript
        it("resets selected checkboxes back to unchecked", function () {
            items[0].checked = true;
            items[1].checked = true;
            checkboxList.reset();
            expect(items[0].checked).toBe(false);
            expect(items[1].checked).toBe(false);
        });

And with a failing test, we put in place the code to make the test pass:

checkboxlist.js

    function reset() {
        $(":checked", container).click();
    }
    return {
        ...
        reset: reset,
        ...
    };

The code now passes, and we can now update the searchresults clearAllClickHandler function:

        function clearAllClickHandler() {
            resetTextField(searchField);
            checkboxList.reset();
        }

Next steps

Now that the uncheckCheckbox and clearAllClickHandler functions have been moved out of searchresults to a better place, that should allow us to more easily remove unwanted template code from the searchresults tests.