10
\$\begingroup\$

This code simulates a polygon (say a triangle) with a person at each vertex, every person is looking at the next adjacent vertex/person. They all start moving towards each other with a constant velocity "trying to chase other". I could not understand this conceptually so I created an applet in Java to help me understand it in physics.

package test;

import java.applet.Applet;
import java.awt.Graphics;
import java.awt.Graphics2D;
import java.awt.geom.GeneralPath;
import java.util.ArrayList;

public class ChasePolygon extends Applet {

    public void paint(Graphics g) {
        final int radius = 200;
        final float velocity = 0.001f;
        final int sides = 6;
        final float theta = (float) (2 * Math.PI / sides);
        final int centerX = 200, centerY = 200;
        ArrayList<Float[]> vertices_loc = new ArrayList<Float[]>(sides);
        for (int i = 0; i < sides; i++) {
            final float pX = (float) (centerX + radius * Math.cos(i * theta));
            final float pY = (float) (centerY + radius * Math.sin(i * theta));
            vertices_loc.add(new Float[] { pX, pY });
        }
        Graphics2D g2 = (Graphics2D) g;
        GeneralPath polyline = new GeneralPath(GeneralPath.WIND_EVEN_ODD, sides);
        polyline.moveTo(vertices_loc.get(0)[0], vertices_loc.get(0)[1]);
        for (int i = 0; i < sides; i++) {
            polyline.lineTo(vertices_loc.get(i)[0], vertices_loc.get(i)[1]);
        }
        polyline.lineTo(vertices_loc.get(0)[0], vertices_loc.get(0)[1]);
        g2.draw(polyline);
        int delta_T = (int) Math
                .floor((radius * Math.sin(theta / 2) / velocity));
        ArrayList<GeneralPath> g_p = new ArrayList<GeneralPath>(sides);
        for (int i = 0; i < sides; i++) {
            g_p.add(new GeneralPath(GeneralPath.WIND_EVEN_ODD, delta_T));
            g_p.get(i).moveTo(vertices_loc.get(i)[0], vertices_loc.get(i)[1]);
        }
        for (int i = 0; i < delta_T; i++) {
            for (int j = 0; j < sides; j++) {
                float dX = 0, dY = 0;
                if (j != sides - 1) {
                    dX = vertices_loc.get(j + 1)[0] - vertices_loc.get(j)[0];
                    dY = vertices_loc.get(j + 1)[1] - vertices_loc.get(j)[1];
                } else {
                    dX = vertices_loc.get(0)[0] - vertices_loc.get(j)[0];
                    dY = vertices_loc.get(0)[1] - vertices_loc.get(j)[1];
                }
                vertices_loc.get(j)[0] += dX * velocity;
                vertices_loc.get(j)[1] += dY * velocity;
                g_p.get(j).lineTo(vertices_loc.get(j)[0],
                        vertices_loc.get(j)[1]);
            }
        }
        for (int i = 0; i < sides; i++) {
            g2.draw(g_p.get(i));
        }

    }
}

Here is the sample output:

\$\hskip1.25in\$enter image description here

Improvements could be (not necessarily these but any you consider):

  • Better way/s to do certain things, specific to programming only (not to my algorithm).
  • My first attempt with Java graphics/Swing etc. so any advice/hints etc.
  • Performance. Yes, it's slow; try running it.
  • It restarts when I change applet size.
  • Fix applet size, etc. as you wish.
\$\endgroup\$
  • \$\begingroup\$ So what are you seeking from your post? This is a really cool applet, but I don't know what you are looking for. \$\endgroup\$ – Evan Bechtol Apr 13 '15 at 17:49
  • 2
    \$\begingroup\$ @EvanBechtol I am not a pro programmer so whenever I code, I ask to review so that I can learn well form you all. My every post revieved excellent answers from various users that is helping me very well. \$\endgroup\$ – RE60K Apr 13 '15 at 17:50
  • 1
    \$\begingroup\$ I would update your questions with specific goals for this question so that you can get some quality answers that pertain to things you are looking for from your code. (e.g. Efficiency improvement, clarity of code, bugs, etc.) \$\endgroup\$ – Evan Bechtol Apr 13 '15 at 17:52
  • \$\begingroup\$ @EvanBechtol tried editing your advice is it good to go? \$\endgroup\$ – RE60K Apr 13 '15 at 18:00
  • \$\begingroup\$ Looks great! Should help you get some specific solutions :) @ADG \$\endgroup\$ – Evan Bechtol Apr 13 '15 at 18:01
13
\$\begingroup\$

I ran through your code, and agree that the output is pretty. I like the idea, and it is a good use-case for a number of exercises. Your code suffers though from a few issues that are awkward.

First up, you have just one real method. Why? You should isolate discrete logic elements in to their own methods. That's the most apparent issue, but we'll get there, but other things get in the way first.

Constants

While we are isolating things, why do you redeclare constant values each time you call paint? Let's take the first chunk of your code:

public class ChasePolygon extends Applet {

    public void paint(Graphics g) {
        final int radius = 200;
        final float velocity = 0.001f;
        final int sides = 6;
        final float theta = (float) (2 * Math.PI / sides);
        final int centerX = 200, centerY = 200;

That should be:

public class ChasePolygon extends Applet {

    private final int radius = 200;
    private final float velocity = 0.001f;
    private final int sides = 6;
    private final float theta = (float) (2 * Math.PI / sides);
    private final int centerX = 200;
    private final int centerY = 200;

    public void paint(Graphics g) {

That's making a bunch of your constants... constants.

Floats

Floats are an awkward value type in Java. You have to keep casting them as floats because the 'natural' floating-point number is a double. I recommend tossing floats out unless you are space-constrained. You are not space constrained, so all your floats should just be doubles. This makes your code a bunch easier to read... for example, lines like:

final float pX = (float) (centerX + radius * Math.cos(i * theta));

become just:

final double pX = centerX + radius * Math.cos(i * theta);

vertices_loc

This ArrayList is a decent concept, but, your code would become a lot easier, and all the 1's and 0's that are scattered though your code would be redundant if you just created two double[] arrays. Consider:

double[] verticesX = new double[sides];
double[] verticesY = new double[sides];

Now you lose your [0] and [1] subscripts for X and Y instead. It is shorter, and even has more meaning. Double-win.

Function Extraction

Right, now it is time for function extraction. First up, extract the polygon draw. We have a trick we can do there too...

Your current polygon draw code is:

GeneralPath polyline = new GeneralPath(GeneralPath.WIND_EVEN_ODD, sides);
polyline.moveTo(vertices_loc.get(0)[0], vertices_loc.get(0)[1]);
for (int i = 0; i < sides; i++) {
    polyline.lineTo(vertices_loc.get(i)[0], vertices_loc.get(i)[1]);
}
polyline.lineTo(vertices_loc.get(0)[0], vertices_loc.get(0)[1]);
g2.draw(polyline);

Note how you start at (moveTo) vertix 0, and your first line draws to (lineTo) vertix 0? You essentially start with a 0-length line, and then you have to fix that with a post-loop line-to vertix 0.

A neater solution would be to just start at the last vertex, and then go from there to the first.... But, put that in a function too. Using my suggestion for the verticesX and verticesY, it becomes (note the sides - 1 moveTo):

private void paintPolygon(Graphics2D g2, double[] verticesX, double[] verticesY) {
    GeneralPath polyline = new GeneralPath(GeneralPath.WIND_EVEN_ODD, sides);
    polyline.moveTo(verticesX[sides - 1], verticesY[sides - 1]);
    for (int i = 0; i < sides; i++) {
        polyline.lineTo(verticesX[i], verticesY[i]);
    }
    g2.draw(polyline);
}

Spirals

The final functional extraction comes from the spiral paths. These should be extracted. Again, using an actual array makes it easier than using an ArrayList. Additionally, the final print loop would be better as a for-each loop. That final method becomes:

private void paintCurves(Graphics2D g2, double[] verticesX, double[] verticesY) {
    GeneralPath[] g_p = new GeneralPath[sides];
    for (int i = 0; i < sides; i++) {
        g_p[i] = new GeneralPath(GeneralPath.WIND_EVEN_ODD, delta_T);
        g_p[i].moveTo(verticesX[i], verticesY[i]);
    }
    for (int i = 0; i < delta_T; i++) {

        for (int j = 0; j < sides; j++) {
            double dX = verticesX[(j + 1) % sides] - verticesX[j];
            double dY = verticesY[(j + 1) % sides] - verticesY[j];
            verticesX[j] += dX * velocity;
            verticesY[j] += dY * velocity;
            g_p[j].lineTo(verticesX[j], verticesY[j]);
        }
    }
    for (GeneralPath gp : g_p) {
        g2.draw(gp);
    }
}

Constant vertices.

Finally, after simplifying things, it became apparent that the initial vertices are constant, and you then overwrite them as you spiral in. This makes it simpler to calculate the vertices once, use those for the polygon, and then use a copy for the spirals. The final paint method is just:

public void paint(Graphics g) {

    Graphics2D g2 = (Graphics2D) g;
    paintPolygon(g2);
    paintCurves(g2, Arrays.copyOf(verticesX, sides), Arrays.copyOf(verticesY, sides));

}

Clean version

All told, I would go with the following:

import java.applet.Applet;
import java.awt.Graphics;
import java.awt.Graphics2D;
import java.awt.geom.GeneralPath;
import java.util.ArrayList;
import java.util.Arrays;

public class ChasePolygon extends Applet {

    private final int radius = 200;
    private final double velocity = 0.001;
    private final int sides = 7;
    private final double theta = 2.0 * Math.PI / sides;
    private final int centerX = 200;
    private final int centerY = 200;
    private final int delta_T;
    private final double[] verticesX = new double[sides];
    private final double[] verticesY = new double[sides];

    public ChasePolygon() {
        delta_T = (int) Math.floor(radius * Math.sin(theta / 2) / velocity);
        for (int i = 0; i < sides; i++) {
            verticesX[i] = centerX + radius * Math.cos(i * theta);
            verticesY[i] = centerY + radius * Math.sin(i * theta);
        }

    }

    public void paint(Graphics g) {

        Graphics2D g2 = (Graphics2D) g;
        paintPolygon(g2);
        paintCurves(g2, Arrays.copyOf(verticesX, sides), Arrays.copyOf(verticesY, sides));

    }

    private void paintCurves(Graphics2D g2, double[] vertX, double[] vertY) {
        GeneralPath[] g_p = new GeneralPath[sides];
        for (int i = 0; i < sides; i++) {
            g_p[i] = new GeneralPath(GeneralPath.WIND_EVEN_ODD, delta_T);
            g_p[i].moveTo(vertX[i], vertY[i]);
        }
        for (int i = 0; i < delta_T; i++) {

            for (int j = 0; j < sides; j++) {
                double dX = vertX[(j + 1) % sides] - vertX[j];
                double dY = vertY[(j + 1) % sides] - vertY[j];
                vertX[j] += dX * velocity;
                vertY[j] += dY * velocity;
                g_p[j].lineTo(vertX[j], vertY[j]);
            }
        }
        for (GeneralPath gp : g_p) {
            g2.draw(gp);
        }
    }

    private void paintPolygon(Graphics2D g2) {
        GeneralPath polyline = new GeneralPath(GeneralPath.WIND_EVEN_ODD, sides);
        polyline.moveTo(verticesX[sides - 1], verticesY[sides - 1]);
        for (int i = 0; i < sides; i++) {
            polyline.lineTo(verticesX[i], verticesY[i]);
        }
        g2.draw(polyline);
    }

}
\$\endgroup\$
  • \$\begingroup\$ As a side note: it would be better for memory to declare dX and dY outside the for loop in the paintCurves method. Otherwise it is re-created every single iteration. Not a big deal, but when extrapolating the program it conserves memory :) Great answer btw! \$\endgroup\$ – Evan Bechtol Apr 13 '15 at 23:56
  • 2
    \$\begingroup\$ @EvanBechtol - Thanks, appreciate the compliment. About the 'declare it outside the loop', that's not going to impact memory allocation. How the java runtime decides to optimize that is hard to predict, and should not matter. The general rule is to declare variables in their narrowest scope. I would not recommend anything different. \$\endgroup\$ – rolfl Apr 14 '15 at 0:00
  • \$\begingroup\$ Nice, I wasn't aware of that :) I'll have to remember this! \$\endgroup\$ – Evan Bechtol Apr 14 '15 at 0:02
  • \$\begingroup\$ @EvanBechtol the dX and dY change with time and much \$\endgroup\$ – RE60K Apr 14 '15 at 10:36

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.