patternjavascriptMinor
Dynamic Radar Chart
Viewed 0 times
chartradardynamic
Problem
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
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
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 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
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 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
Solution
I assume you are talking mostly about the huge statement in the
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
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.Code Snippets
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 };
}Context
StackExchange Code Review Q#158260, answer score: 2
Revisions (0)
No revisions yet.