patternjavascriptMinor

CSS & JS piechart

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

piechartcssstackoverflow

Problem

I made a pieChart function thanks to the main CSS properties rotate:xdeg, border-radius:100% and overflow:hidden. That helped me to make rotating quarter pies of any size.

Here is the code:

function getRandomColor() {
  var letters = '0123456789ABCDEF'.split('');
  var color = '#';
  for (var i = 0; i  2 ? getRandomColor() : colorArray[m];
      for (var j = 1; j

div.piechart,
.piechart .frame_0,
.piechart .frame_2 {
width: 80px;
height: 80px;
}

.piechart .frame_1,
.piechart .square {
width: 40px;
height: 40px;
}

div.piechart {
position: relative;
float: left;
margin-right: 10px;
text-align: center;
}

.piechart .frame_0 {
position: absolute;
}

.piechart .frame_1 {
overflow: hidden;
}

.piechart .square {
border-radius: 100% 0 0 0;
}

.piechart .nodata {
border-radius: 100%;
background-color: #FFD419;
}

.piechart .label {
position: absolute;
text-align: center;
width: 20PX;
height: 20PX;
line-height: 20PX;
font-size: 7pt;
color: white;
font-weight: bold;
}

`

I am searching for any other code optimization.

JSfiddle

Solution

I am looking at a particular function you have

function drawQuarter(percent, angle, color) {
  var widthPercent = 90 - percent;
  var widthAngle = angle - 90 + percent;
  var output = '';
  output += '';
  output += '';
  output += '';
  output += '';
  output += '';
  output += '';
  output += '';

  return output;
}

I just see a lot of redundancy with output += and then all the `

 as well.  I would use the JavaScript concatenation functionality like this:

function drawQuarter(percent, angle, color) {
var widthPercent = 90 - percent;
var widthAngle = angle - 90 + percent;
var output = '' +
    ' ' +
    '' +  
    '   ';
return output;
}


also

for (var j = 1; j <= quarterQuantityArray[m]; j++) {
    if (j != quarterQuantityArray[m]) {
      output += drawQuarter(90, rotation, color);
      rotation += 90;
    }
    if (j == quarterQuantityArray[m]) {
      var angle = percent[m] * 360 % 90;
      angle = angle == 0 ? 90 : angle; //In case of 25%, 50%, 75% and 100%
      output += drawQuarter(angle, rotation, color);
      rotation += angle;
    }
  }


both of these cannot be true at the same time

j != quarterQuantityArray[m]

j == quarterQuantityArray[m]




and we only need to check for one of these, not both.  This should be an if/else statement like this

for (var j = 1; j <= quarterQuantityArray[m]; j++) {
    if (j != quarterQuantityArray[m]) {
        output += drawQuarter(90, rotation, color);
        rotation += 90;
    } else {
        var angle = percent[m] * 360 % 90;
        angle = angle == 0 ? 90 : angle; //In case of 25%, 50%, 75% and 100%
        output += drawQuarter(angle, rotation, color);
        rotation += angle;
    }
}


I know that normally you shouldn't have magic numbers floating around but when you are setting a style value, that really doesn't count as a magic number because it says right there what it is,  I am talking about this piece of your code, right here:

HTMLoutput = '';
dtop = 30;
dleft = 16;
HTMLoutput += 'No data';
return HTMLoutput;


specifically the

dtop and the dleft variables, I don't even see them var`'ed anywhere... I would just write it like this

return = 'No data';

Make sure that you are consistent with your bracketing around your if/else statements and also watch to make sure that your indentation is correct, it could cause issues when you are trying to troubleshoot issues later if everything is not consistent.

Code Snippets

function drawQuarter(percent, angle, color) {
  var widthPercent = 90 - percent;
  var widthAngle = angle - 90 + percent;
  var output = '<div class="frame_0" style="transform: rotate(' + widthAngle + 'deg);">';
  output += '<div class="frame_1">';
  output += '<div class="frame_2" style="transform: rotate(' + widthPercent + 'deg);">';
  output += '<div class="square" style="background-color:' + color + '">';
  output += '</div>';
  output += '</div>';
  output += '</div>';
  output += '</div>';

  return output;
}

function drawQuarter(percent, angle, color) {
var widthPercent = 90 - percent;
var widthAngle = angle - 90 + percent;
var output = '<div class="frame_0" style="transform: rotate(' + widthAngle + 'deg);">' +
    '<div class="frame_1"> <div class="frame_2" style="transform: rotate(' + widthPercent + 'deg);">' +
    '<div class="square" style="background-color:' + color + '">' +  
    '</div> </div> </div> </div>';
return output;
}

for (var j = 1; j <= quarterQuantityArray[m]; j++) {
    if (j != quarterQuantityArray[m]) {
      output += drawQuarter(90, rotation, color);
      rotation += 90;
    }
    if (j == quarterQuantityArray[m]) {
      var angle = percent[m] * 360 % 90;
      angle = angle == 0 ? 90 : angle; //In case of 25%, 50%, 75% and 100%
      output += drawQuarter(angle, rotation, color);
      rotation += angle;
    }
  }

for (var j = 1; j <= quarterQuantityArray[m]; j++) {
    if (j != quarterQuantityArray[m]) {
        output += drawQuarter(90, rotation, color);
        rotation += 90;
    } else {
        var angle = percent[m] * 360 % 90;
        angle = angle == 0 ? 90 : angle; //In case of 25%, 50%, 75% and 100%
        output += drawQuarter(angle, rotation, color);
        rotation += angle;
    }
}

HTMLoutput = '<div class="frame_square nodata"></div>';
dtop = 30;
dleft = 16;
HTMLoutput += '<div class="label" style="top:' + dtop + 'px; left:' + dleft + 'px; line-height:normal;width:50px;">No data</div>';
return HTMLoutput;

Context

StackExchange Code Review Q#119209, answer score: 4

Revisions (0)

No revisions yet.