Search code examples
javascriptjqueryevalflatpickr

replace the eval with something safer


I created a calendar with the flatpickr.js library. I want to enable only the dates that are present in the arreys dateCorfu, dateZante and datePag selected from a dropdown (with the attribute name="destinazione").

I'm not a coder but I wrote the below code using the eval method, this is working but I'm sure that this is the wrong way.. maybe there is a better way to manipulate the result of function(date). So is there a way to make this to works without using eval?

flatpickr("#data-partenza", {
    locale:'it', 
    minDate: "2020-07-16",
    enable: [
        function(date) {
            // return true to enable
            var drop_destinazione = jQuery('[name="destinazione"]').val();
            var result = '';
            var dateCorfu = ["2020-07-16","2020-07-23","2020-07-30","2020-08-06","2020-08-13","2020-08-20"];
            var dateZante = ["2020-07-17","2020-07-24","2020-07-31","2020-08-07","2020-08-14"];
            var datePag = ["2020-07-18","2020-07-25","2020-08-01","2020-08-08"];
            var slice = 'date.toISOString().slice(0,10)';
            var i;
            if(drop_destinazione == 'Corfù'){
                for (i = 0; i < dateCorfu.length; i++) {
                    result += (slice + '==' + '"' + dateCorfu[i] + '"' + '||');
                }
                return eval(result.slice(0, -2));
            }
            else if(drop_destinazione == 'Zante'){
                for (i = 0; i < dateZante.length; i++) {
                    result += (slice + '==' + '"' + dateZante[i] + '"' + '||');
                }
                return eval(result.slice(0, -2));
            }
            else if(drop_destinazione == 'Pag'){
                for (i = 0; i < datePag.length; i++) {
                    result += (slice + '==' + '"' + datePag[i] + '"' + '||');
                }
                return eval(result.slice(0, -2));
            }
            else {
                console.log('no destination selected')
            }
        }
    ],
    dateFormat: "d-m-Y",
    disableMobile: true,
});

Solution

  • since you are only doing if-validations, why dont you just immediately evaluate and accumulate at each result += ..?

    result = false;
    for (...) {
        if (condition) result = true;
    }
    return result;
    

    or even better: exit early like this:

    result = true;
    for (...) {
        if (condition) return true;
    }
    return false;
    

    applied to your original code this would make your function look like this:

    function(date) {
        var drop_destinazione = jQuery('[name="destinazione"]').val();
        var dateCorfu = ["2020-07-16","2020-07-23","2020-07-30",
                         "2020-08-06","2020-08-13","2020-08-20"];
        var dateZante = ["2020-07-17","2020-07-24","2020-07-31",
                         "2020-08-07","2020-08-14"];
        var datePag = ["2020-07-18","2020-07-25","2020-08-01","2020-08-08"];
        var slice = date.toISOString().slice(0, 10);
        if (drop_destinazione == 'Corfù') {
            for (var i = 0; i < dateCorfu.length; i++) {
                if (slice == dateCorfu[i]) return true;
            }
        }
        else if(drop_destinazione == 'Zante') {
            for (var i = 0; i < dateZante.length; i++) {
                if (slice == dateZante[i]) return true;
            }
        }
        else if(drop_destinazione == 'Pag') {
            for (var i = 0; i < datePag.length; i++) {
                if (slice == datePag[i]) return true;
            }
        }
        else { console.log('no destination selected'); }
        return false;
    }
    

    however there are steps you can take to make your code less repetitive:

    function(date) {
        var drop_destinazione = jQuery('[name="destinazione"]').val();
        var slice = date.toISOString().slice(0, 10);
        var allowed = {
            Corfù: ["2020-07-16","2020-07-23","2020-07-30",
                    "2020-08-06","2020-08-13","2020-08-20"],
            Zante: ["2020-07-17","2020-07-24","2020-07-31",
                    "2020-08-07","2020-08-14"],
            Pag: ["2020-07-18","2020-07-25","2020-08-01",
                  "2020-08-08"],
        };
        if (allowed[drop_destinazione] == undefined) {
            console.log('no destination selected');
            return false;
        }
        return allowed[drop_destinazione].indexOf(slice) !== -1;
    }
    

    of course the property name "Corfù" does not look too nice, however "drop_destinazione" seems to be a dropdown, these have both a value and a label (see https://developer.mozilla.org/en-US/docs/Web/HTML/Element/select). you should make the values simple strings like "corfu":

    <select>
        <option value="corfu">Corfù</option>
        ...
    </select>
    

    if you do it like this, you can make the allowed index look nicer:

    var allowed = {
        corfu: ["2020-07-16","2020-07-23","2020-07-30",
                "2020-08-06","2020-08-13","2020-08-20"],
        zante: ["2020-07-17","2020-07-24","2020-07-31",
                "2020-08-07","2020-08-14"],
        pag: ["2020-07-18","2020-07-25","2020-08-01",
              "2020-08-08"],
    };
    

    hope this helps!