3
\$\begingroup\$

I want to organize some JavaScript code using module pattern.

The scenario is as follows: Having the following html table created dynamically

<table class="table table-hover">
    <tbody>
        <tr>
            <td colspan="2" style="border-top: 0">2018-03-27</td>
        </tr>
        <tr>
            <td data-id="8" data-user="Yesenia" data-classroom="D101" data-date="2018-03-27" style="vertical-align: middle;">
                <a href="http://localhost:8080/ds/request/show/8">Yesenia en D101</a>
            </td>
            <td width="1">
                <form action="http://localhost:8080/ds/request/delete/8" method="POST" value="DELETE" onsubmit="if (!confirm('¿Estás seguro?')) return false;" style="margin: 0;">
                    <input name="_method" value="DELETE" type="hidden">
                    <button type="submit" class="btn btn-link btn-small">
                        <i class="icon-trash"></i>
                    </button>
                </form>
            </td>
        </tr>
        <tr>
            <td colspan="2">2018-03-26</td>
        </tr>
        <tr>
            <td data-id="1" data-user="user user" data-classroom="D101" data-date="2018-03-26" style="vertical-align: middle;">
                <a href="http://localhost:8080/ds/request/show/1">user user en D101</a>
            </td>
            <td width="1">
                <form action="http://localhost:8080/ds/request/delete/1" method="POST" value="DELETE" onsubmit="if (!confirm('¿Estás seguro?')) return false;" style="margin: 0;">
                    <input name="_method" value="DELETE" type="hidden">
                    <button type="submit" class="btn btn-link btn-small">
                        <i class="icon-trash"></i>
                    </button>
                </form>
            </td>
        </tr>
        <tr>
            <td data-id="2" data-user="user user" data-classroom="D102" data-date="2018-03-26" style="vertical-align: middle;">
                <a href="http://localhost:8080/ds/request/show/2">user user en D102</a>
            </td>
            <td width="1">
                <form action="http://localhost:8080/ds/request/delete/2" method="POST" value="DELETE" onsubmit="if (!confirm('¿Estás seguro?')) return false;" style="margin: 0;">
                    <input name="_method" value="DELETE" type="hidden">
                    <button type="submit" class="btn btn-link btn-small">
                        <i class="icon-trash"></i>
                    </button>
                </form>
            </td>
        </tr>
        <tr>
            <td data-id="7" data-user="Yesenia" data-classroom="D103" data-date="2018-03-26" style="vertical-align: middle;">
                <a href="http://localhost:8080/ds/request/show/7">Yesenia en D103</a>
            </td>
            <td width="1">
                <form action="http://localhost:8080/ds/request/delete/7" method="POST" value="DELETE" onsubmit="if (!confirm('¿Estás seguro?')) return false;" style="margin: 0;">
                    <input name="_method" value="DELETE" type="hidden">
                    <button type="submit" class="btn btn-link btn-small">
                        <i class="icon-trash"></i>
                    </button>
                </form>
            </td>
        </tr>
        <tr>
            <td data-id="9" data-user="Cesar" data-classroom="D101" data-date="2018-03-26" style="vertical-align: middle;">
                <a href="http://localhost:8080/ds/request/show/9">Cesar en D101</a>
            </td>
            <td width="1">
                <form action="http://localhost:8080/ds/request/delete/9" method="POST" value="DELETE" onsubmit="if (!confirm('¿Estás seguro?')) return false;" style="margin: 0;">
                    <input name="_method" value="DELETE" type="hidden">
                    <button type="submit" class="btn btn-link btn-small">
                        <i class="icon-trash"></i>
                    </button>
                </form>
            </td>
        </tr>
    </tbody>
</table>

And an input to filter the table

<input id="filter" placeholder="Filtrar...">

The following JavaScript logic is implemented to allow filter the table

const dataset = Array.from(document.querySelectorAll('[data-id]')).map(td => Object.assign({}, td.dataset));
const trigger = document.querySelector('#filter');
const serverURL = window.serverURL;

function applyFilter(event) {
    const results = dataset.filter(data => data.user.toLowerCase().includes(event.target.value));

    render(results);
}

function render(dataset) {
    let date = null;
    const rows = [];

    dataset.forEach((data, index) => {
        if (data.date !== date) {
            rows.push(renderDateRow(data.date, index));
        }

        rows.push(renderDataRow(data));

        date = data.date;
    });

    document.querySelector('table tbody').innerHTML = rows.join('');
}

function renderDateRow(date, index) {
    return `<tr>
        <td colspan="2" ${index === 0 ? 'style="border-top: 0"' : ''}>${date}</td>
    </tr>`;
}

function renderDataRow(data) {
    return `<tr>
        <td
            data-id="${data.id}"
            data-user="${data.user}"
            data-classroom="${data.classroom}"
            data-date="${data.date}"
            style="vertical-align: middle;">
            <a href="${serverURL}/request/show/${data.id}">${data.user} en ${data.classroom}</a>
        </td>
        <td width="1">
            <form
                action="${serverURL}/request/delete/${data.id}"
                method="POST"
                value="DELETE"
                onSubmit="if (!confirm('¿Estás seguro?')) return false;"
                style="margin: 0;">
                <input type="hidden" name="_method" value="DELETE">
                <button type="submit" class="btn btn-link btn-small">
                    <i class="icon-trash"></i>
                </button>
            </form>
        </td>
    </tr>`;
}

trigger.addEventListener('keyup', applyFilter);

I use Module pattern in order to organize the code, the final implementation looks like this:

const FilterModule = (() => {
    const dataset = Array.from(document.querySelectorAll('[data-id]')).map(td => Object.assign({}, td.dataset));

    function loadServerURL() {
        return window.serverURL;
    }

    function applyFilter(event) {
        const results = dataset.filter(data => data.user.toLowerCase().includes(event.target.value));

        render(results);
    }

    function loadTrigger() {
        document.querySelector('#filter').addEventListener('keyup', applyFilter);
    }

    function render(dataset) {
        let date = null;
        const rows = [];

        dataset.forEach((data, index) => {
            if (data.date !== date) {
                rows.push(renderDateRow(data.date, index));
            }

            rows.push(renderDataRow(data));

            date = data.date;
        });

        document.querySelector('table tbody').innerHTML = rows.join('');
    }

    function renderDateRow(date, index) {
        return `<tr>
            <td colspan="2" ${index === 0 ? 'style="border-top: 0"' : ''}>${date}</td>
        </tr>`;
    }

    function renderDataRow(data) {
        const serverURL = loadServerURL();

        return `<tr>
            <td
                data-id="${data.id}"
                data-user="${data.user}"
                data-classroom="${data.classroom}"
                data-date="${data.date}"
                style="vertical-align: middle;">
                <a href="${serverURL}/request/show/${data.id}">${data.user} en ${data.classroom}</a>
            </td>
            <td width="1">
                <form
                    action="${serverURL}/request/delete/${data.id}"
                    method="POST"
                    value="DELETE"
                    onSubmit="if (!confirm('¿Estás seguro?')) return false;"
                    style="margin: 0;">
                    <input type="hidden" name="_method" value="DELETE">
                    <button type="submit" class="btn btn-link btn-small">
                        <i class="icon-trash"></i>
                    </button>
                </form>
            </td>
        </tr>`;
    }

    function init() {
        loadTrigger();
    }

    return {
        init
    };
})();

FilterModule.init();

I thank you for any comments in order to better organize the code

\$\endgroup\$
  • 1
    \$\begingroup\$ Welcome to Code Review! I hope you get great answers. \$\endgroup\$ – Phrancis Mar 27 '18 at 4:29
2
\$\begingroup\$

Your module is pretty good, and most of the code is really clever.

I have just a couple of suggestions for you to help you enhancing your design.

Coupling

The most important issue I see in your code is that your module is strongly coupled with the table you're rendering and the way you bind the data.

That mean you have impact in the module, if you are going to change the table layout or the data loading.

Here the point that are affected:

const dataset = Array.from(document.querySelectorAll('[data-id]')).map(td => Object.assign({}, td.dataset));

As you see not only you have to fix the selector of the data, but even the way you find in the DOM element and of course the DOM element...

function renderDateRow(date, index) {...

And

function renderDataRow(data) {...

Those funcions need to know exactly how to render a table row, so if you need a table without filter, you should duplicate that code. And if you have to change the table layout you have to change that code either. That is an issue for the maintenance of the module.

How to overcome

Now I propose some solution to that coupling.

You should change your init() function to accept some dependencies like:

function init(loadData, tableRenderer)

Both loadData() and tableRenderer() have to be function you're going to inject in your code.

The loadData() is simple, it just return a list of records.

tableRenderer(data) should just accept a new record set of data: the one you filtered.

Non necessary functions

The event binding is trivial in your case, so you should rid the loadTrigger() function and have the code directly in the init function:

function init(loadData, tableRenderer, filterSelector) {
    const filterElement = document.querySelector(filterSelector || '#filter');

    filterElement.addEventListener('keyup', applyFilter);
}

You can even have advantage of the injected function ad have a factory function for your applyFilter():

function makeFilterFrom(dataset, renderer) {
    return function (event) {
         renderer(applyFilter(event, dataset));
    }
}

And so the bind line became:

...
filterElement.addEventListener('keyup', makeFilterFrom(loadData(), tableRenderer));
...

Cleanup

As your module register an event listener, you should have a function that get rid of that in order to avoid memory leaks.

const listeners = [];

...
const handler = makeFilterFrom(loadData(), tableRenderer);
filterElement.addEventListener('keyup', handler);
listeners.push({element: filterElement, event: 'keyup', fn: handler});

...
function cleanup() {
    while(listeners.length) {
        let listener = listener.pop();
        listener.element.removeEventListener(listener.event, listener.fn);
    }
}

And then you should expose that function to allow the user of your module to use it.

Naming

The FilterModule is not a good name as you use module that is redundant.

A better name could be ContentFilter or TableFilter.

Naming things is always hurd, but try to have a better name is always woth.

Here is a rewrite of the module:

// That should be a constant, you don't need a function
const serverURL = window.serverURL;

function loadData() {
  const dataTable = document.querySelectorAll('[data-id]');
  return Array.from(dataTable).map(td => Object.assign({}, td.dataset));
}

function render(dataset) {
    let date = null;
    const rows = [];

    dataset.forEach((data, index) => {
        if (data.date !== date) {
            rows.push(renderDateRow(data.date, index));
        }

        rows.push(renderDataRow(data));

        date = data.date;
    });

    document.querySelector('table tbody').innerHTML = rows.join('');
}

function renderDateRow(date, index) {
    return `<tr>
        <td colspan="2" ${index === 0 ? 'style="border-top: 0"' : ''}>${date}</td>
    </tr>`;
}

function renderDataRow(data) {
    return `<tr>
        <td
            data-id="${data.id}"
            data-user="${data.user}"
            data-classroom="${data.classroom}"
            data-date="${data.date}"
            style="vertical-align: middle;">
            <a href="${serverURL}/request/show/${data.id}">${data.user} en ${data.classroom}</a>
        </td>
        <td width="1">
            <form
                action="${serverURL}/request/delete/${data.id}"
                method="POST"
                value="DELETE"
                onSubmit="if (!confirm('¿Estás seguro?')) return false;"
                style="margin: 0;">
                <input type="hidden" name="_method" value="DELETE">
                <button type="submit" class="btn btn-link btn-small">
                    <i class="icon-trash"></i>
                </button>
            </form>
        </td>
    </tr>`;
}

// Here starts your actual module, the previous code is just 
// from other modules
const ContentFilter = (() => {
    const listeners = [];

    function applyFilter(filterValue, dataset) {
      return dataset.filter(record =>
        record.user.toLowerCase().includes(filterValue));
    }

    function makeFilterFrom(dataset, renderer) {
      return function (event) {
           renderer(applyFilter(event.target.value, dataset));
      }
    }

    // An utility to forge the object
    function listenerFrom(element, event, fn) {
      return {element: element, event: event, fn: fn};
    }

    function init(loadData, tableRenderer, filterSelector) {
      const filterElement = document.querySelector(filterSelector || '#filter');
      const handler = makeFilterFrom(loadData(), tableRenderer);
      listeners.push(listenerFrom(filterElement, 'keyup', handler));
      filterElement.addEventListener('keyup', handler);
    }

    function cleanup () {
      while(listeners.length) {
        let listener = listener.pop();
        listener.element.removeEventListener(listener.event, listener.fn);
      }
    }

    return {
        init,
        cleanup
    };
})();

ContentFilter.init(loadData, render, '');

// Rememeber to handle the cleanup on page unload or if you 
// redraw all the piece of page where you have the filter
// module.
\$\endgroup\$
  • \$\begingroup\$ Very good observations, I totally agree with the idea. thanks for the time \$\endgroup\$ – user615274 Mar 28 '18 at 1:22

Your Answer

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

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