6
\$\begingroup\$

I've created a dynamic radar chart which allows manipulation of points by clicking. My code works, but I'm sure the large amount of if.. else if statements can be processed in a loop, because it is clearly a repetition.

I thought about how I could use a loop to do it, but couldn't figure it out because of the way the coordinates of points are located.

My main concern is the if.. else if because it looks scary.

Here is live demo jsfiddle;

and here is the whole javascript:

var canv = document.getElementById('canvas');
var canv1 = document.getElementById('canvas1');
var point_xy = document.getElementById('point_xy');
var temp = [];
var point_xy_cords = [
  []
];
var pentagon_one = 24;
var pentagon_two = 18;
var pentagon_three = 12;
var pentagon_four = 6;
var pentagon_five = 0;
var circles = [];
var contx = canv.getContext('2d');
var contx1 = canv1.getContext('2d');
var j = 0;
var offsetX = canv1.offsetLeft;
var offsetY = canv1.offsetTop;
contx.clearRect(0, 0, canv.width, canv.height);

function drawShape(ctx, x, y, points, radius1, radius2, alpha0) {
  //points: number of points (or number of sides for polygons)
  //radius1: "outer" radius of the star
  //radius2: "inner" radius of the star (if equal to radius1, a polygon is drawn)
  //angle0: initial angle (clockwise), by default, stars and polygons are 'pointing' up
  var i, angle, radius;
  if (radius2 !== radius1) {
    points = 2 * points;
  }
  for (i = 0; i <= points; i++) {
    angle = i * 2 * Math.PI / points - Math.PI / 2 + alpha0;
    radius = i % 2 === 0 ? radius1 : radius2;
    temp = [(x + radius * Math.cos(angle)), (y + radius * Math.sin(angle))];
    ctx.lineTo(temp[0, 0], temp[0, 1]);
    point_xy_cords[j++] = temp;

  }
  point_xy.textContent = point_xy_cords;
}

function style(ctx, fill) {
  ctx.strokeStyle = "#bada55";
  ctx.lineWidth = 2;
  if (fill) {
    ctx.fillStyle = "rgba(35, 63, 57, 0.8)";
    ctx.fill();
  } else {
    ctx.stroke()
  }

  //contx.fill();
}

var radius = 5;

function points(ctx, x, y) {
  ctx.beginPath();
  ctx.arc(x, y, radius, 0, 2 * Math.PI, false);
  ctx.fillStyle = 'gold';
  ctx.stroke();
  ctx.lineWidth = 1;
  ctx.strokeStyle = '#003300';
  ctx.stroke();
}

var Circle = function(x, y, radius) {
  this.left = x - radius;
  this.top = y - radius;
  this.right = x + radius;
  this.bottom = y + radius;
};

var radius_size = 100;

function drawBackgroundPentagons(ctx) {
  for (var x = 0; x <= 5; x++) {
    ctx.beginPath();
    drawShape(ctx, 120, 120, 5, radius_size, radius_size, 0);
    style(ctx);
    ctx.closePath();
    radius_size = radius_size - 20;
  }

}

function drawMainPentagon(ctx, points) {

  contx1.beginPath();
  contx1.moveTo(points[0][0], points[0][1]);
  for (var x = 1; x <= 5; x++) {
    ctx.lineTo(points[x][0], points[x][1]);
  }
  style(ctx, "fill");
  ctx.closePath();
}
drawBackgroundPentagons(contx);

//draw main Pentagon
//0, 6, 12, 18, 24
test = [];
var start = 24;
for (var x = 0; x <= 5; x++) {
  test[x] = point_xy_cords[start];
  start++;
}
drawMainPentagon(contx1, test);

//Draw all points
var n = 30;
while (n > 0) {
  var circle = new Circle(point_xy_cords[n][0], point_xy_cords[n][1], 8);
  circles.push(circle);
  points(contx1, point_xy_cords[n][0], point_xy_cords[n][1]);
  n--;
}

function handleMouseDown(e, message) {
  mouseX = parseInt(e.pageX - offsetX);
  mouseY = parseInt(e.pageX - offsetY);
  point_xy.textContent = (message);
}

canv1.onmousedown = function(e) {
  //var clickedX = e.pageX - this.offsetLeft;
  //var clickedY = e.pageY - this.offsetTop;
  var x = parseInt(e.pageX - 20);
  var y = parseInt(e.pageY - 20);
  var pointX = test[1][0];
  var pointY = test[1][1];
  var dx = x - (pointX);
  var dy = y - (pointY);
  var distsq = dx * dx + dy * dy;
  var rsq = 25;
  var array = [4, 3, 5, 3, 0];

  //handleMouseDown(e, x + " x | " + pointX + " pointx |" + pointY + " pointy | " + y + " y | " + dx + " dx | " + dy + " dy|" + distsq + " distsq | " + offsetX);


  //contx1.clearRect(0, 0, canv.width, canv.height);
  //drawMainPentagon(contx1, test);
  //
  for (var i = 0; i < circles.length; i++) {
    if (x < circles[i].right && x > circles[i].left && y > circles[i].top && y < circles[i].bottom) {
      //alert ('clicked number ' + (i + 1));
      handleMouseDown(e, "point clicked: " + i + " | test object: " + test);
      if (i == 6) {
        test[0] = point_xy_cords[pentagon_one];
        test[5] = point_xy_cords[pentagon_one];
      } else if (i == 12) {
        test[0] = point_xy_cords[pentagon_two];
        test[5] = point_xy_cords[pentagon_two];
      } else if (i == 18) {
        test[0] = point_xy_cords[pentagon_three];
        test[5] = point_xy_cords[pentagon_three];
      } else if (i == 24) {
        test[0] = point_xy_cords[pentagon_four];
        test[5] = point_xy_cords[pentagon_four];
      } else if (i == 25) {
        test[0] = point_xy_cords[pentagon_five];
        test[5] = point_xy_cords[pentagon_five];
      }

      else if (i == 5) {
        test[1] = point_xy_cords[pentagon_one + 1];
      }
            else if (i == 11) {
        test[1] = point_xy_cords[pentagon_two + 1];
      }
      else if (i == 17) {
        test[1] = point_xy_cords[pentagon_three + 1];
      }
      else if (i == 23) {
        test[1] = point_xy_cords[pentagon_four + 1];
      }
      else if (i == 29) {
        test[1] = point_xy_cords[pentagon_five + 1];
      }

      else if (i == 4) {
        test[2] = point_xy_cords[pentagon_one + 2];
      }
            else if (i == 10) {
        test[2] = point_xy_cords[pentagon_two + 2];
      }
      else if (i == 16) {
        test[2] = point_xy_cords[pentagon_three + 2];
      }
      else if (i == 22) {
        test[2] = point_xy_cords[pentagon_four + 2];
      }
      else if (i == 28) {
        test[2] = point_xy_cords[pentagon_five + 2];
      }

      else if (i == 3) {
        test[3] = point_xy_cords[pentagon_one + 3];
      }
            else if (i == 9) {
        test[3] = point_xy_cords[pentagon_two + 3];
      }
      else if (i == 15) {
        test[3] = point_xy_cords[pentagon_three + 3];
      }
      else if (i == 21) {
        test[3] = point_xy_cords[pentagon_four + 3];
      }
      else if (i == 27) {
        test[3] = point_xy_cords[pentagon_five + 3];
      }

      else if (i == 2) {
        test[4] = point_xy_cords[pentagon_one + 4];
      }
            else if (i == 8) {
        test[4] = point_xy_cords[pentagon_two + 4];
      }
      else if (i == 14) {
        test[4] = point_xy_cords[pentagon_three + 4];
      }
      else if (i == 20) {
        test[4] = point_xy_cords[pentagon_four + 4];
      }
      else if (i == 26) {
        test[4] = point_xy_cords[pentagon_five + 4];
      }
    }
  }
  contx1.clearRect(0, 0, canv.width, canv.height);
  var n = 30;
    while (n > 0) {
    points(contx1, point_xy_cords[n][0], point_xy_cords[n][1]);
    n--;
    }
  drawMainPentagon(contx1, test);
};

I am looking for some direction as to how I can avoid the massive repetition that is taking place, and increase the efficiency of the overall code.

\$\endgroup\$
  • 1
    \$\begingroup\$ Note that you can make live demos directly in Stack Exchange by pressing Ctrl-M in the question editor. \$\endgroup\$ – 200_success Mar 20 '17 at 6:26
  • \$\begingroup\$ I have rolled back the last edit. Please see what you may and may not do after receiving answers. \$\endgroup\$ – Heslacher Mar 22 '17 at 8:33
  • \$\begingroup\$ @Heslacher Would my edit qualify as a new question in Stackoverflow, or codereview? \$\endgroup\$ – almost a beginner Mar 22 '17 at 8:37
  • 1
    \$\begingroup\$ If your new code is having issues then Stack Overflow would be the place to ask a new question. For questions on Code Review you should consider to wait some more days because answers here may take some more time. Asking a new question is ok but you should include a link to this question and show in the question that it is a follow up question. \$\endgroup\$ – Heslacher Mar 22 '17 at 8:41
2
\$\begingroup\$

I assume you are talking mostly about the huge statement in the onmouseclick handler? I would start by making your Circle class a bit smarter, give it the information it needs and move some of the logic there. More specifically I would let each point know which pentagon axis it is on and it's index from the center. Then add methods to Circle like containsPoint(x,y), draw(context) and most importantly clicked(). Your code would then look something like:

function onmouseclicked() {
  for (var i = 0; i < circles.length; i++) {
    var circle = circles[i];
    if (circle.containsPoint(clickedX, clickedY) {
      circle.clicked();
      return;
    }
  }
}

Circle.prototype.clicked = function() {
  test[axis] = {x: this.x, y: this.y };
}

I would start here, see how ti affects your code and then come back for a second review if you want.

Note: A lot of your intermediate variables like point_xy_coords, pentagon_one are unnecessary. If you do keep point_xy_cords make it more structure. Currently you are doing something like point_xy_coords[axis*5+index], instead do an array of arrays point_xy_coords[axis][index]. Also I wouldn't make your temp array have 6 points. Just draw the extra line in your drawMainPentagon routine.

\$\endgroup\$
  • \$\begingroup\$ ahh, it makes sense. I will follow your guide and get back to you when I'm done. Thanks. \$\endgroup\$ – almost a beginner Mar 22 '17 at 1:18
  • \$\begingroup\$ Hi again, I managed to edit my code with your help, could you please review it one more time. \$\endgroup\$ – almost a beginner Mar 22 '17 at 8:29
  • \$\begingroup\$ Are your changes somewhere I can see them? \$\endgroup\$ – Marc Rohloff Mar 22 '17 at 16:08
  • \$\begingroup\$ I just posted a question at StackOverflow because I'm having issues figuring out which corner to change after implementing your code review. When I fix that issue, I will post it here again for another review. Here's the link: stackoverflow.com/questions/42965497/… \$\endgroup\$ – almost a beginner Mar 23 '17 at 1:14

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.