4
\$\begingroup\$

I created this little JavaScript application which gets data from a post request and uses this data to show a line chart with the total income over a year and a bar chart for the daily income. The data comes as a JSON object.

For example:

dates:["01.01."],["02.01."],["03.01."],...
dailyBudgets:[3],[1],[2],[6],... 
totalBudgets: [3],[4],[6],[12]...

There is also a jQuery slider to dynamically show the data in between two dates. I implemented a stack to store the unused values in an array. You have to manipulate the original data array of the charts in order for the animations to work properly.

Source:

Can someone have a look at this, especially the self implemented stack? Everything is working properly so far, but since I'm not that experienced with JavaScript, I wanted to get some expertise on how to optimize this.

var lineChart;
var barChart;
var totalBudgets;
var dailyBudgets;
var dates; 

$.post(APIURL, function(result) {
    if (result.success) {
        totalBudgets = result.totalBudgets;
        dailyBudgets = result.dailyBudgets;
        dates = result.dates;

        //Create line chart
        var ctxLine = document.getElementById("lineChart").getContext('2d');
        lineChart = new Chart(ctxLine,  {
            type : 'line',
            data : {
                labels : dates,
                datasets : [ {
                    label : '€',
                    data : totalBudgets
                } ]
            }
        });

        //Create bar chart
           var ctxBar = document.getElementById("barChart").getContext('2d');
            barChart = new Chart(ctxBar, {
                type : 'bar',
                data : {
                    labels : dates,
                    datasets : [ {
                        label : '€',
                        data : dailyBudgets
                    }]
                }
            });
        }
    }, 'json').success(
        function(result) {
            if (result.success) {
                bottomStack = [];
                topStack = [];
                lastBottomIndex = 0;
                lastTopIndex = dates.length - 1;

                updateData();

                //Create slider
                $("#slider").slider({
                    range : true,
                    min : 0,
                    max : dates.length - 1,
                    values : [ 0, dates.length ],
                    slide : function(event, ui) {
                        //Check which handle was moved
                        if (ui.handleIndex == 0) {
                            if (ui.value < lastBottomIndex) {
                                for (i = 0; i < lastBottomIndex - ui.value; i++) {
                                    dates.unshift(bottomStack.pop());
                                    totalBudgets.unshift(bottomStack.pop());
                                    dailyBudgets.unshift(bottomStack.pop());
                                }
                            } else {
                                for (i = 0; i < ui.value - lastBottomIndex; i++) {
                                    bottomStack.push(dailyBudgets.shift());
                                    bottomStack.push(totalBudgets.shift());
                                    bottomStack.push(dates.shift());
                                }
                            }
                            lastBottomIndex = ui.value;
                        } else {
                            if (ui.value > lastTopIndex) {
                                for (i = 0; i < ui.value - lastTopIndex; i++) {
                                    dates.push(topStack.pop());
                                    totalBudgets.push(topStack.pop());
                                    dailyBudgets.push(topStack.pop());
                                }
                            } else {
                                for (i = 0; i < lastTopIndex - ui.value; i++) {
                                    topStack.push(dailyBudgets.pop());
                                    topStack.push(totalBudgets.pop());
                                    topStack.push(dates.pop());
                                }
                            }
                            lastTopIndex = ui.value;
                        }
                        updateData();
                    }
                });
        } else {
            console.log(result.error);
        }
    });

function updateData() {
    barChart.update();
    lineChart.update();
    $("#startDateLabel").html(dates[0]);
    $("#endDateLabel").html(dates[dates.length - 1]);
}

HTML:

<div id="barChartTab">
    <canvas id="barChart"></canvas>
</div>
<div id="lineChartTab">
    <canvas id="lineChart"></canvas>
</div>
<div class="sliderContainer">
    <div id="slider"></div>
    <div class="sliderDates">
        <div id="startDate"><a id="startDateLabel"></a></div>
        <div id="endDate"><a id="endDateLabel"></a></div>
    </div>
</div>

Note: I've removed some trivial pieces of the code. If there is missing some information, feel free to ask.

\$\endgroup\$
  • \$\begingroup\$ This is a lot of code to look at without being able to execute it. You should consider taking everything out except for the XHR response where your stack lives. \$\endgroup\$ – I wrestled a bear once. Dec 12 '17 at 19:29
  • \$\begingroup\$ Is the API external or do you have control of the data structures? It seems like all that looping is something that might be better done on the server-side unless you don't have control over that... \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Dec 13 '17 at 0:07
  • \$\begingroup\$ @SamOnela I have control over the API, but I don't think you can handle this on the server-side since every user is allowed to change the slider (and therefore the array). I definitely don't want to have a permanent open connection to the server. \$\endgroup\$ – Ian Fako Dec 14 '17 at 14:29
  • \$\begingroup\$ @Iwrestledabearonce. I think the visual representation of the the data is very important here, sorry if I was not able to make a working example yet, probably updating the post when I get back to work. \$\endgroup\$ – Ian Fako Dec 14 '17 at 14:33
2
\$\begingroup\$

So, your stack is storing three different types of things: dates, totalBudgets, and dailyBudgets. These things are repeating in that specific order in a single stack. This seems like a very fragile system, if one thing is added or removed without the following two your software will break. Since these three items are always added and removed together you should group them so each set of three is always together and your order is never broken.

if (ui.value < lastBottomIndex) {
  for (i = 0; i < lastBottomIndex - ui.value; i++) {
    var set = bottomStack.pop();
    dates.unshift(set.date);
    totalBudgets.unshift(set.totalBudget);
    dailyBudgets.unshift(set.dailyBudget);
  }
} else {
  for (i = 0; i < ui.value - lastBottomIndex; i++) {
    bottomStack.push({
      date: dates.shift(),
      dailyBudget: dailyBudgets.shift(),
      totalBudget: totalBudgets.shift()
    });
  }
}
\$\endgroup\$

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.