3
\$\begingroup\$

I'm new to javascript programming and not sure if I'm coding correctly.

For testing myself I created a small JS application.

The idea of app is:

List of athletes, grouped by country and the number of medals won. The data is provided as a JSON file. The application must have a pure MVC structure.

Could you please review the code of my app and point me to the good and bad sides of the application code, style, stucture etc. How can I improve this app?

Here I put all files of my app.

package.json

{
  "name": "testJsApp",
  "version": "0.0.0",
  "description": "Test JS App",
  "scripts": {
    "start": "lite-server"
  },
  "license": "ISC",
  "devDependencies": {
    "lite-server": "^2.3.0"
  }
}

index.html

<!DOCTYPE html>
<html lang="ru">
<head>
    <meta charset="UTF-8">
    <title>Test test</title>
    <meta name="viewport" content="width=device-width, initial-scale=1.0">

    <link href="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.7/css/bootstrap.min.css" rel="stylesheet">
    <link rel="stylesheet" href="style.css">

    <script src="https://code.jquery.com/jquery-3.2.1.min.js"></script>
    <script src="https://cdnjs.cloudflare.com/ajax/libs/handlebars.js/4.0.8/handlebars.min.js"></script>
</head>

<body>
    <section id="app">
        <select class="countries"></select>
        <script class="countries-template" type="text/x-handlebars-template">
            {{#list nav}}
                <option value="{{value}}">{{title}}</option>
            {{/list}}
        </script>


        <div class="medals">
            <span class="btn btn-s btn-warning" data-medal-type="gold">gold</span>
            <span class="btn btn-s btn-default" data-medal-type="silver">silver</span>
            <span class="btn btn-s btn-danger"  data-medal-type="bronze">bronze</span>
            <span class="btn btn-s btn-info active">all</span>
        </div>


        <div class="pages">
            <button class="btn btn--prev">prev</button>
            <span class="pages__info">
                <span class="item-no"></span> of <span class="items-count"></span>
            </span>
            <button class="btn btn--next">next</button>
        </div>


        <div class="template-holder"></div>

        <script class="person-template" type="text/x-handlebars-template">
            <div class="person well">
                <h4>Person info:</h4>
                <p class="person__name"><span class="person__label">Name: </span>{{name}}</p>
                <p class="person__country"><span class="person__label">Country: </span>{{country}}</p>
            </div>
        </script>
    </section>


    <script src="model.js"></script>
    <script src="app.js"></script>
</body>
</html>

style.css

#app { padding: 10px; width: 400px; }

.countries { margin-bottom: 20px; width: 100%; }

.medals { margin-bottom: 20px; }

.medals .btn { border: 3px solid transparent; }

.medals .btn.active { box-shadow: inset 2px 2px 5px rgba(0,0,0,.5); }

.pages { margin-bottom: 20px; }

data.json

[
    {"name":"Titus Tindall"     , "medal":"g", "country":"amber"},
    {"name":"Tyrell Toenjes"    , "medal":"b", "country":"amber"},
    {"name":"Amanda Avant"      , "medal":"b", "country":"amber"},
    {"name":"Mickey Mcfarlain"  , "medal":"s", "country":"amber"},
    {"name":"Roseline Rothermel", "medal":"s", "country":"amber"},
    {"name":"Kanisha Knop"      , "medal":"b", "country":"amber"},
    {"name":"Dylan Dankert"     , "medal":"b", "country":"amber"},
    {"name":"Takako Traverso"   , "medal":"g", "country":"amber"},
    {"name":"Perry Parkerson"   , "medal":"s", "country":"amber"},

    {"name":"Ethan Eisner"      , "medal":"b", "country":"shire"},
    {"name":"Stefan Schartz"    , "medal":"b", "country":"shire"},
    {"name":"Love Lail"         , "medal":"g", "country":"shire"},
    {"name":"Linsey Lytch"      , "medal":"s", "country":"shire"},
    {"name":"Trent Torrens"     , "medal":"b", "country":"shire"},
    {"name":"Minda Maple"       , "medal":"g", "country":"shire"},
    {"name":"Leandro Leleux"    , "medal":"g", "country":"shire"},

    {"name":"Cary Cordray"      , "medal":"s", "country":"earthsea"},
    {"name":"Moon Murrill"      , "medal":"b", "country":"earthsea"},
    {"name":"Vance Verduzco"    , "medal":"g", "country":"earthsea"},
    {"name":"Bernard Berner"    , "medal":"s", "country":"earthsea"},

    {"name":"Randal Rispoli"    , "medal":"g", "country":"narnia"},
    {"name":"Mercedez Monarrez" , "medal":"s", "country":"narnia"},
    {"name":"Sol Spinella"      , "medal":"b", "country":"narnia"},
    {"name":"Mireille Mullings" , "medal":"g", "country":"narnia"},
    {"name":"Genesis Gainey"    , "medal":"s", "country":"narnia"},
    {"name":"Annelle Ahlstrom"  , "medal":"b", "country":"narnia"}
]

model.js

'use strict';

var DATA_URL = 'data.json';

/** Person ---------------------------------------------------- */
class Person {
    constructor(personObj) {
        this.name = personObj.name;
        this.country = personObj.country;
        this.medal = personObj.medal;
    }

    set name(name) {
        this._name = name;
        return this;
    }

    set country(country) {
        this._country = country;
        return this;
    }

    set medal(medal) {
        this._medal = medal;
        return this;
    }


    get name() {
        return this._name;
    }


    get country() {
        return this._country;
    }


    get medal() {
        return this._medal;
    }


    getFieldByName(fieldName) {
        let vFieldName;
        if (this.hasOwnProperty(fieldName)) {
            vFieldName = fieldName;
        } else if (this.hasOwnProperty(`_${fieldName}`)) {
            vFieldName = `_${fieldName}`;
        } else {
            console.log(this);
            throw {code:20001, message:`Field ${fieldName} not found`};
        };

        return this[fieldName];
    }
};


/** Persons ---------------------------------------------------- */
class Persons {
    constructor() {
        this._personsData = null;
        this._personsListAll = [];
        this._personsList = [];
    }


    loadData() {
        return $.when($.getJSON(DATA_URL));
    }


    getPersonsData() {
        return this._personsData;
    }


    init(data) {
        this._personsData = data;
        this._personsListAll = [];
        this._personsList = [];

        for (let i = 0; i < this._personsData.length; i++) {
            let personTmp = new Person(this._personsData[i]);
            this._personsListAll.push(personTmp);
            this._personsList = this._personsListAll.filter(() => true);
        };
    }


    getPerson(id) {
        return this._personsList[id];
    }


    _filterByFieldValue(fieldName, fieldValue) {
        let vFieldName, vFieldValue;

        if (arguments.length === 1 && typeof(fieldName) === 'object') {
            let argObj = fieldName;
            vFieldName = argObj['name'];
            vFieldValue = argObj['value'];
        } else {
            vFieldName = fieldName;
            vFieldValue = fieldValue;
        };

        this._personsList = this._personsList.filter(item => item.getFieldByName(vFieldName) === vFieldValue);
    }


    filterByFieldValue(fieldName, fieldValue) {
        this._personsList = this._personsListAll.filter(() => true);
        if (arguments.length === 1) {
            if (Array.isArray(fieldName)) {
                let arr = fieldName;
                for (let i = 0; i < arr.length; i++) {
                    this._filterByFieldValue(arr[i]);
                }
            } else if (typeof(fieldName) === 'object') {
                let argObj = fieldName;
                this._filterByFieldValue(argObj);
            }
        } else {
            this._filterByFieldValue(fieldName, fieldValue);
        }
    }


    get count() {
        return this._personsList.length;
    }


    get countriesList() {
        const names = this._personsListAll.map(item => item.country);
        return names.filter((item, i, ar) => ar.indexOf(item) === i).sort();
    }
};


/** CurrentPerson ---------------------------------------------------- */
class CurrentPerson {
    constructor(persons) {
        this._currentPersonId = 0;
        this.persons = persons;
    }


    set currentId(id) {
        this._currentPersonId = id;
    }


    get currentId() {
        return this._currentPersonId;
    }


    get() {
        return this.persons.getPerson(this._currentPersonId);
    }


    next() {
        if (this._currentPersonId + 1 < this.persons.count) {
            this._currentPersonId++;
            return true;
        };
        return false;
    }


    prev() {
        if (this._currentPersonId > 0) {
            this._currentPersonId--;
            return true;
        };
        return false;
    }
};

app.js

'use strict';

(function() {
    function forEach(ctx, func) {
        try {
            return ctx.forEach(func);
        } catch(e) {
            return Array.prototype.forEach.call(ctx, func);
        }
    }


    class App {
        constructor() {
            this._appElem = document.querySelector('#app');
            this._currPerson = null;
            this._selectedMedal = null;
            this._selectedCountry = null;

            this.regHelpers();
            this.addSelectorEventListener();
            this.addNavBtnListeners();
            this.addMedalsEventListeners();
            this.fillCountriesSelector();
            this.renderPerson();
        }


        get selectedCountry() {
            return this._selectedCountry || all;
        }


        get selectedMedal() {
            return this._selectedMedal || all;
        }


        clickSelector(event) {
            this._selectedCountry = event.target.value;
            this.filterPersons();
            this.renderPerson();
        }


        addSelectorEventListener() {
            forEach(this._appElem.querySelectorAll('.countries'), item => item.addEventListener('change', this.clickSelector.bind(this)));
        }


        _removeBtnsActiveClass() {
            forEach(this._appElem.querySelectorAll('.medals .btn'), item => item.classList.remove('active'));
        }


        clickMedalBtn(event) {
            this._removeBtnsActiveClass();

            const attr = event.target.getAttribute('data-medal-type');

            if (attr) {
                this._selectedMedal = attr[0];
            } else {
                this._selectedMedal = null;
            };

            this.filterPersons();
            this.renderPerson();

            event.target.classList.add('active');
        }


        addMedalsEventListeners() {
            forEach(this._appElem.querySelectorAll('.medals span'), item => item.addEventListener('click', this.clickMedalBtn.bind(this)));
        }


        clickNavBtn(event) {
            let flag;

            if (event.target.classList.contains('btn--next')) {
                flag = currentPerson.next();
            } else {
                flag = currentPerson.prev();
            }

            if (flag) {
                this.renderPerson();
            }
        }


        addNavBtnListeners() {
            forEach(this._appElem.querySelectorAll('.pages .btn'), item => item.addEventListener('click', this.clickNavBtn.bind(this)));
        }


        filterPersons() {
            const dataFilter = [];

            if (!!this._selectedCountry && this._selectedCountry.toLowerCase() != 'all') {
                dataFilter.push({name:'country', value:this._selectedCountry});
            };

            if (this._selectedMedal && this._selectedMedal.toLowerCase() != 'a') {
                dataFilter.push({name:'medal', value:this._selectedMedal});
            }

            persons.filterByFieldValue(dataFilter);

            currentPerson.currentId = 0;
        }


        get currentPersonsId() {
            return currentPerson.currentId;
        }


        get personsCount() {
            return persons.count;
        }


        get templateObj() {
            return { name: this._currPerson.name, country: this._currPerson.country };
        }


        get countriesList() {
            return persons.countriesList.map(item => { return { value:item, title: item } });
        }


        renderPerson() {
            this._currPerson = currentPerson.get();

            let template = Handlebars.compile(this._appElem.querySelector('.person-template').innerHTML);
            let html = template(this.templateObj);
            this._appElem.querySelector('.template-holder').innerHTML = html;

            this._appElem.querySelector('.pages__info .item-no').innerHTML = this.currentPersonsId + 1;
            this._appElem.querySelector('.pages__info .items-count').innerHTML = this.personsCount;
        }


        fillCountriesSelector() {
            let countriesList = this.countriesList;
            countriesList.unshift({value:null, title:'all countries'});

            let template = Handlebars.compile(this._appElem.querySelector('.countries-template').innerHTML);

            let html = template({ nav: countriesList });
            this._appElem.querySelector('.countries').innerHTML = html;
        }


        regHelpers() {
            Handlebars.registerHelper('list', function(context, options) {
                var ret = '';
                for(var i=0, j=context.length; i<j; i++) {
                    ret = ret + options.fn(context[i]);
                };
                return ret;
            });
        }
    };


    /** ----- RUN APP ----------------------------------------------------------- */
    var app, currentPerson;

    var persons = new Persons();

    persons.loadData()
           .then(data => persons.init(data))
           .then(() => currentPerson = new CurrentPerson(persons))
           .then(() => app = new App());
})();

app screen shot

enter image description here

update

I've made some modifications:
    1. removed direct indications of field names in the Person.getFieldByName() function,
    2. removed all links to global variables from model.js,
    3. wrapped content of app.js into an Immediately-Invoked Function Expression,
    4. used promise when loading data at app initialisation

Is there anything else to be fixed?

\$\endgroup\$

Your Answer

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

Browse other questions tagged or ask your own question.