2
\$\begingroup\$

I have a working application, which fetches list of events (seminars) from rest API. Functionality is quite simple: display list of seminars with possibility to filter upcoming/occured/all. The application can be found here.

Inspired by some Vue docs I've created such App object which contains store, some helper functions (getDate, getState) and function, which returns seminar object with transformed and ready to be displayed data:

var App = {
    store: new Storage("/wp-json/wp/v2/seminar"),
    //Parse date string and get date in different formats
    getDate: function (str, part) {
        "use strict";
        var y = str.slice(0, 4);
        var m = str.slice(4, 6);
        var d = str.slice(6, 8);

        var month = ["января", "февраля", "марта", "апреля", "мая", "июня", "июля", "августа", "сентября", "октября", "ноября", "декабря"];

        if (part === "d") {
            return d;
        } else if (part === "m") {
            return m;
        } else if (part === "mm") {
            return month[parseInt(m) - 1];
        } else if (part === "y") {
            return y;
        } else {
            return y + "-" + m + "-" + d;
        }
    },

    //get state of seminar
    getState: function (str) {
        "use strict";
        return (new Date() > new Date(this.getDate(str)))
            ? "upcoming"
            : "occured";
    },

    //get object with all necessary data to display
    getSeminar: function (seminar) {
        "use strict";

        return {
            id: seminar.id,
            title: seminar.title.rendered,
            date: new Date(this.getDate(seminar.acf.date)),
            displayDate: this.getDate(seminar.acf.date),
            //occured: (new Date() > new Date(this.getDate(seminar.acf.date)),
            state: this.getState(this.getDate(seminar.acf.date)),
            time: seminar.acf.time,
            day: this.getDate(seminar.acf.date, "d"),
            month: this.getDate(seminar.acf.date, "mm"),
            year: this.getDate(seminar.acf.date, "y"),
            reporter: seminar.acf.reporter,
            place: seminar.acf.place
        };
    }
};

And here is code to create vue instance. I assign functions from App object to vue instance methods and in created () function I fetch data and populate seminars attribute.

var vue = new Vue({
    el: '#app',
    data: {
        seminars: [],
        show: 'all'
    },
    computed: {
        title: function () {
            "use strict";
            return (this.show)
                ? "Предстоящие семинары"
                : "Архив семинаров";
        },
        count: function () {
            "use strict";
            var cnt = 0;
            var show = this.show;
            this.seminars.forEach(function (seminar) {
                if (seminar.state === show || show === 'all') {
                    cnt += 1;
                }
            });

            return cnt;
        }
    },
    methods: {
        getSeminar: App.getSeminar,
        getDate: App.getDate,
        getState: App.getState
    },
    created: function () {
        "use strict";
        var vm = this;
        App.store.fetch().then(function (result) {
            result.forEach(function (seminar) {
                vm.seminars.push(vm.getSeminar(seminar));
            });
            //app.seminars = seminars;
        });
    }
});

There is also some code in my application, but it's not related to my question at the moment, but I'll mention it, just for completeness:

Storage class (actually looks, that I don't really need it here, because it does really nothing helpful, I use only the fetch() method of it):

/*global window, xhr*/
var Storage = function (url) {
    "use strict";
    this.baseUrl = url;

    this.get = function (id) {
        return xhr.get("get", this.baseUrl + "/" + id);
    };

    this.fetch = function () {
        return xhr.get("get", url);
    };
};

window.Storage = Storage;

And xhr object, which implements http request:

var xhr = {
    get: function (method, url) {
        "use strict";
        return new Promise(function (resolve, reject) {
            var req = new XMLHttpRequest();
            req.open(method, url);
            req.onload = function () {
                if (this.status >= 200 && this.status < 300) {
                    resolve(JSON.parse(req.response));
                } else {
                    reject({
                        status: this.status,
                        statusText: req.statusText
                    });
                }
            };
            req.onerror = function () {
                reject({
                    status: this.status,
                    statusText: req.statusText
                });
            };
            req.send();
        });

    }

};


window.xhr = xhr;

My general question is - do I need an App object, or what is best practice to implement some business logic in vue.js application? Maybe it would be better to implements data conversion and transformation inside of vue methods?

\$\endgroup\$
  • \$\begingroup\$ @t3chb0t, my mistake to say that. It works, except latest safari in iOS, so it doesn't really matter in this case. \$\endgroup\$ – soeik Jan 31 '18 at 12:54
  • 1
    \$\begingroup\$ @t3chb0t, Ok, thank you. I'll try to improve description. \$\endgroup\$ – soeik Jan 31 '18 at 12:59
  • \$\begingroup\$ The previous title was better... this one sounds like you app isn't working agian. Just replace some data with seminar data and it's ok. \$\endgroup\$ – t3chb0t Jan 31 '18 at 13:28
  • 1
    \$\begingroup\$ @t3chb0t, Ok, I'll leave it in peace. ^) \$\endgroup\$ – soeik Jan 31 '18 at 13:31
1
\$\begingroup\$

Main question

do I need an App object, or what is best practice to implement some business login in vue.js application?

Well, it depends on how large of an application you create. For the sample code given, you could just move the methods from App to the Vue instance, unless that feels like too much coupling of the view layer and controller code.

What you describe sounds like Vuex. On the When should I used it? section of the documentation, it mentions that "A simple global event bus may be all you need."

Other feedback

"use strict"; in every function

Are there any functions that should not have strict mode invoked? If not, then just add one "use script"; to the beginning of each file. Or are there external scripts that don't have scrict mode invoked?

naming of Storage

The Storage class doesn't appear to actually store data - it merely fetches data (like you typed: "actually looks, that I don't really need it here, because it does really nothing helpful, I use only fetch() method of it").

xhr class

The xhr class could be replaced by the Fetch API, or a similar tool like reqwest, superagent, etc.

parseInt() without specifying radix

App.getDate() has the following line:

return month[parseInt(m) - 1];

"Always specify [the radix] to eliminate reader confusion and to guarantee predictable behavior"1

 return month[parseInt(m, 10) - 1];

Assigning properties on window

The lines like the following:

window.xhr = xhr;

Should be superfluous, unless of course xhr is defined within a function (e.g. an IIFE)...

Functional approach

You mentioned "trying to do the same in functional style" - perhaps you have already made changes like below but if not, it could really shorten things.

For instance, the created method could be shortened to below:

App.store.fetch().then(function (result) {
    this.seminars = this.seminars.concat(result.map(this.getSeminar));
}.bind(this)); 

Notice that Function.bind() is used to pass the this context into the callback handler, removing the need to assign var vm = this;.

Or instead of using .concat(), .unshift() could be used with the spread operator:

App.store.fetch().then(function (result) {
    this.seminars.unshift(...result.map(this.getSeminar));
}.bind(this));

And an arrow function can remove the need to bind the function to this:

App.store.fetch().then(result => this.seminars.unshift(...result.map(this.getSeminar)));

1https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/parseInt#Parameters

\$\endgroup\$
  • \$\begingroup\$ Thanks for review and comments! I agree, for small app like this it would be easier to add all methods to the Vue instance, but I wanted to keep it clean as possible. I've read about vuex and event bus in documentation, but still don't know how to use first one in real life. And after 20 days, since I've posted it, I feel that I don't like this code and also see a lot of things to improve. Now I'm trying to do the same in functional style and it looks much more clean and better. \$\endgroup\$ – soeik Feb 23 '18 at 13:43
  • \$\begingroup\$ Great - I added a few more feeback items. I thought about suggesting using date.toLocaleDateString('ru-Cyrl', { month: 'long' }) to get the month but it isn't in genitive form like you have it. If that worked, you wouldn't need to store the array var months... \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Feb 23 '18 at 17:05
  • 1
    \$\begingroup\$ Thanks a lot for a valuable remarks. I'll try to gather it all, and refactor my code. When I mentioned FP, I meant that I'm totally reworking it with Kefir.js and React. \$\endgroup\$ – soeik Feb 23 '18 at 20:12

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.