HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Javascript bar graph representation using CSS

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
bargraphcssjavascriptusingrepresentation

Problem

This js was written strictly for a visualization of a graph redesign included in a stats report within our game systems. It all works properly, but I know there has to be a much neater way to organize this. I have some ideas, but I don't quite know how to approach it. Any guidance would be greatly appreciated.

```
var barContainer = document.getElementById('bar-graph');
var barCount = 15;

for (index = 0; index ';
barContainer.appendChild(liBar);
}

//this is for visualization. Sets height of each bar, then adds a margin to the top of each based on height, in order to keep each bar flushed with the bottom of the graph.
var list = document.getElementsByClassName("bar");
var percentGroup = new Array();

for (index = 0; index < list.length; ++index) {
var bar = list[index];
var originalHeight = $(bar).height();
$(bar).css( "height", originalHeight - (index * 10) );
var barHeight = $(bar).height();
var percentage = barHeight / originalHeight;
percentGroup.push(percentage);
var containerHeight = $('.graph').height();
var marginTop = containerHeight - barHeight;
$(bar).css('margin-top', marginTop);
var graph = $('#bar-graph');
var graphWidth = graph.width();
var barWidth = $(bar).width();
var marginRight = ( graphWidth - (barWidth * barCount) ) / ( barCount - 1 );
$(".graph ul li").css('margin-right', marginRight);
$(".graph ul li:nth-last-child(2)").css('margin-right', (marginRight - 1) );
}

//this function takes the decimal value percentages generated above, turns them into integers, and multiplies them by 100. It also creates the containers that house the percentages
var val = document.getElementById('valContainer');

for (index = 0; index < percentGroup.length; ++index) {
var bar = percentGroup[index];
bar = Math.round(bar * 100);
var li = document.createElement('li');

Solution

Too many temporary variables

Try to reduce the number of local temporary variables as they make the code muddy and unnecessary complicated and difficult to change.

Example, these four lines could probably be reduced to a single line:

var barHeight = $(bar).height();
...
var containerHeight = $('.graph').height();
var marginTop = containerHeight - barHeight;
$(bar).css('margin-top', marginTop);


Could be:

$(bar).css('margin-top', $('.graph').height() - $(bar).height());


Delegate tasks to functions

It is not a bad thing to sometimes delegate to functions for clearity at the exepence of minor performance hit:

var graph = $('#bar-graph');
var graphWidth = graph.width();
var barWidth = $(bar).width();
var marginRight = ( graphWidth - (barWidth * barCount) ) / ( barCount - 1 );
$(".graph ul li").css('margin-right', marginRight);
$(".graph ul li:nth-last-child(2)").css('margin-right', (marginRight - 1) );


Could become:

$(".graph ul li").css('margin-right', getRightMargin(bar, barCount));
$(".graph ul li:nth-last-child(2)").css('margin-right', (getRightMargin(bar, barCount) - 1) );
...
function getRightMargin(bar, barCount) {
    var graphWidth = $('#bar-graph').width();
    var barWidth = $(bar).width();
    return ( graphWidth - (barWidth * barCount) ) / ( barCount - 1 );
}


This way the margin calculation will be done twice, but the performance implication in this case is negligible, the code however will be less noisy and more approachable.

Code Snippets

var barHeight = $(bar).height();
...
var containerHeight = $('.graph').height();
var marginTop = containerHeight - barHeight;
$(bar).css('margin-top', marginTop);
$(bar).css('margin-top', $('.graph').height() - $(bar).height());
var graph = $('#bar-graph');
var graphWidth = graph.width();
var barWidth = $(bar).width();
var marginRight = ( graphWidth - (barWidth * barCount) ) / ( barCount - 1 );
$(".graph ul li").css('margin-right', marginRight);
$(".graph ul li:nth-last-child(2)").css('margin-right', (marginRight - 1) );
$(".graph ul li").css('margin-right', getRightMargin(bar, barCount));
$(".graph ul li:nth-last-child(2)").css('margin-right', (getRightMargin(bar, barCount) - 1) );
...
function getRightMargin(bar, barCount) {
    var graphWidth = $('#bar-graph').width();
    var barWidth = $(bar).width();
    return ( graphWidth - (barWidth * barCount) ) / ( barCount - 1 );
}

Context

StackExchange Code Review Q#129877, answer score: 2

Revisions (0)

No revisions yet.