14
\$\begingroup\$

I was actually pretty proud of this programming project that I came across in this book I'm working through. I only really had problems figuring out the formula for detecting whether or not the two circles had intersecting paths. I'm still getting the hang of object-oriented programming, primarily when and where to use static, private, public, etc... but I think I'm on the right path. Let me know what you think.

enter image description here enter image description here

import javafx.application.Application;
import javafx.event.ActionEvent;
import javafx.event.EventHandler;
import javafx.geometry.HPos;
import javafx.geometry.Insets;
import javafx.geometry.Pos;
import javafx.scene.Scene;
import javafx.scene.control.Button;
import javafx.scene.control.Label;
import javafx.scene.control.TextField;
import javafx.scene.layout.GridPane;
import javafx.scene.layout.HBox;
import javafx.scene.layout.Pane;
import javafx.scene.layout.VBox;
import javafx.scene.paint.Color;
import javafx.scene.paint.Paint;
import javafx.scene.shape.Circle;
import javafx.stage.Stage;

/**
 * Created by John on 7/28/2014.
 */
public class Project16_8 extends Application {
    private static String intersectAnswer = "Two circles intersect? ";
    private static Label intersect;

    @Override // Override the start method in the Application class
    public void start(Stage primaryStage) {
        // Initialize the intersect Label
        intersect = new Label(intersectAnswer + "No");

        // Pane to hold the circles
        Pane circlePane = new Pane();
        circlePane.setPadding(new Insets(5));
        CircleObject circle1 = new CircleObject(52.0, 60.0, 30);
        CircleObject circle2 = new CircleObject(180.0, 56.0, 40);
        circlePane.getChildren().addAll(circle1, circle2);

        // Circle 1 info Pane
        Pane circle1Pane = new Pane();
        CircleInfo circle1Info = new CircleInfo("Enter circle 1 info: ",
            52.0, 60.0, 30);
        circle1Pane.getChildren().add(circle1Info);

        // Circle 2 info Pane
        Pane circle2Pane = new Pane();
        CircleInfo circle2Info = new CircleInfo("Enter circle 2 info: ",
            180.0, 56.0, 40);
        circle2Pane.getChildren().add(circle2Info);

        // HBox to hold the two circle info pane's
        HBox circleInfoHBox = new HBox(5);
        circleInfoHBox.setPadding(new Insets(5));
        circleInfoHBox.getChildren().addAll(circle1Pane, circle2Pane);

        // Redraw Circles button and set ActionEvent Handler
        Button redrawCircles = new Button("Redraw Circles");
        EventHandler<ActionEvent> handler = e -> {
            // Set circle1 variables
            circle1.setCenterX(circle1Info.getCenterX());
            circle1.setCenterY(circle1Info.getCenterY());
            circle1.setRadius(circle1Info.getRadius());

            // Set circle2 variables
            circle2.setCenterX(circle2Info.getCenterX());
            circle2.setCenterY(circle2Info.getCenterY());
            circle2.setRadius(circle2Info.getRadius());

            // Check to see if the circles intersect
            double circle1X = circle1.getCenterX();
            double circle1Y = circle1.getCenterY();
            double circle1Radius = circle1.getRadius();
            double circle2X = circle2.getCenterX();
            double circle2Y = circle2.getCenterY();
            double circle2Radius = circle2.getRadius();

            if (checkIntersection(circle1X, circle1Y, circle1Radius,
                    circle2X, circle2Y, circle2Radius)) {
                intersect.setText(intersectAnswer + "Yes");
            } else {
                intersect.setText(intersectAnswer + "No");
            }
        };
        redrawCircles.setOnAction(handler);
        circle1Info.getTfCenterX().setOnAction(handler);
        circle1Info.getTfCenterY().setOnAction(handler);
        circle1Info.getTfRadius().setOnAction(handler);
        circle2Info.getTfCenterX().setOnAction(handler);
        circle2Info.getTfCenterY().setOnAction(handler);
        circle2Info.getTfRadius().setOnAction(handler);

        // VBox to hold everything
        VBox pane = new VBox();
        pane.setPadding(new Insets(2));
        pane.setStyle("-fx-background-color: white");
        pane.setAlignment(Pos.CENTER);
        pane.getChildren().addAll(intersect, circlePane, circleInfoHBox,
                redrawCircles);

        // Create a scene and add it to the stage
        Scene scene = new Scene(pane);
        primaryStage.setTitle("IntersectingCircles"); // Set the stage title
        primaryStage.setScene(scene); // Place the scene in the stage
        primaryStage.show(); // Display the stage
    }

    /** Compares the two circles to make sure they don't intersect */
    public boolean checkIntersection(double circle1X, double circle1Y,
        double circle1Radius, double circle2X, double circle2Y,
        double circle2Radius) {
        // dx and dy are the vertical and horizontal distances
        double dx = circle2X - circle1X;
        double dy = circle2Y - circle1Y;

        // Determine the straight-line distance between centers.
        double d = Math.sqrt((dy * dy) + (dx * dx));

        // Check Intersections
        if (d > (circle1Radius + circle2Radius)) {
            // No Solution. Circles do not intersect
            return false;
        } else if (d < Math.abs(circle1Radius - circle2Radius)) {
            // No Solution. one circle is contained in the other
            return false;
        } else {
            return true;
        }

    }

    /** Create a Circle object and set it's editable attributes */
    public class CircleObject extends Pane {
        // Data Fields
        private Circle circle;
        private double radius;
        private double centerX;
        private double centerY;
        private Paint fill;
        private Paint stroke;

        /** Create Default Constructor */
        public CircleObject () {
            // Set data fields
            this.centerX = 52.0;
            this.centerY = 60.0;
            this.radius = 30;

            // Paint the Circle
            paintCircle();
        }

        /** Create Secondary Constructor */
        public CircleObject (double centerX, double centerY, double radius) {
            // Set data fields
            this.centerX = centerX;
            this.centerY = centerY;
            this.radius = radius;
            this.fill = null;
            this.stroke = Color.BLACK;

            // Paint the Circle
            paintCircle();
        }

        /** Get centerX */
        public double getCenterX() {
            return this.centerX;
        }

        /** Set centerX */
        public void setCenterX(double centerX) {
            this.centerX = centerX;
            this.circle.setCenterX(this.centerX);
        }

        /** Get centerY */
        public double getCenterY() {
            return this.centerY;
        }

        /** Set centerY */
        public void setCenterY(double centerY) {
            this.centerY = centerY;
            this.circle.setCenterY(this.centerY);
        }

        /** Get radius */
        public double getRadius() {
            return this.radius;
        }

        /** Set radius */
        public void setRadius(double radius) {
            this.radius = radius;
            this.circle.setRadius(this.radius);
        }

        /** Get Fill */
        public Paint getFill() {
            return this.fill;
        }

        /** Set Fill */
        public void setFill(Paint fill) {
            this.fill = fill;
            this.circle.setFill(fill);
        }

        /** Get Stroke */
        public Paint getStroke() {
            return this.stroke;
        }

        /** Set Stroke */
        public void setStroke(Paint stroke) {
            this.stroke = stroke;
            this.circle.setStroke(this.stroke);
        }

        /** Paint the circle object */
        public void paintCircle() {
            // Create a circle object
            this.circle = new Circle();
            this.circle.setCenterX(this.centerX);
            this.circle.setCenterY(this.centerY);
            this.circle.setRadius(this.radius);
            this.circle.setFill(this.fill);
            this.circle.setStroke(this.stroke);
            getChildren().add(this.circle);
        }
    }

    /** Create a Circle info control VBox */
    public class CircleInfo extends VBox {
        // Data Fields
        private String circleLabel;
        private double centerX;
        private double centerY;
        private double radius;

        // Text Fields
        private TextField tfCenterX;
        private TextField tfCenterY;
        private TextField tfRadius;

        /** Default Constructor */
        public CircleInfo() {
            // Set Data Fields
            this.circleLabel = "Enter circle Info: ";
            this.centerX = 52.0;
            this.centerY = 60.0;
            this.radius = 30;

            // Paint the circleInfo VBox
            paintCircleInfo();
        }

        /** Create Secondary Constructor */
        public CircleInfo(String circleLabel, double centerX,
                          double centerY, double radius) {
            // Set Data Fields
            this.circleLabel = circleLabel;
            this.centerX = centerX;
            this.centerY = centerY;
            this.radius = radius;

            // Paint the circleInfo VBox
            paintCircleInfo();
        }

        /** Get Center X */
        public double getCenterX() {
            return this.centerX = Double.parseDouble(tfCenterX.getText());
        }

        /** Set Center X */
        public void setCenterX(double centerX) {
            this.centerX = centerX;
            tfCenterX.setText(String.format("%.1f", this.centerX));
        }

        /** Get Center Y */
        public double getCenterY() {
            return this.centerY = Double.parseDouble(tfCenterY.getText());
        }

        /** Set Center Y */
        public void setCenterY(double centerY) {
            this.centerY = centerY;
            tfCenterY.setText(String.format("%.1f", this.centerY));
        }

        /** Get Radius */
        public double getRadius() {
            return this.radius = Double.parseDouble(tfRadius.getText());
        }

        /** Set Radius */
        public void setRadius(double radius) {
            this.radius = radius;
            tfRadius.setText(String.format("%.1f", this.radius));
        }

        /** Get tfCenterX */
        public TextField getTfCenterX() {
            return this.tfCenterX;
        }

        /** Get tfCenterY */
        public TextField getTfCenterY() {
            return this.tfCenterY;
        }

        /** Get tfRadius */
        public TextField getTfRadius() {
            return this.tfRadius;
        }

        /** Paint the circleInfo VBox */
        public void paintCircleInfo() {
            // Create TextFields
            this.tfCenterX = new TextField();
            this.tfCenterX.setPrefColumnCount(2);
            this.tfCenterX.setText(String.format("%.1f", this.centerX));
            this.tfCenterY = new TextField();
            this.tfCenterY.setPrefColumnCount(2);
            this.tfCenterY.setText(String.format("%.1f", this.centerY));
            this.tfRadius = new TextField();
            this.tfRadius.setPrefColumnCount(2);
            this.tfRadius.setText(String.format("%.1f", this.radius));

            // Create Labels, add TextFields, set content to the right
            Label lbInfo = new Label(circleLabel);
            lbInfo.setAlignment(Pos.CENTER);
            Label lbCenterX = new Label("Center x: ");
            lbCenterX.setAlignment(Pos.CENTER_LEFT);
            Label lbCenterY = new Label("Center y: ");
            lbCenterY.setAlignment(Pos.CENTER_LEFT);
            Label lbRadius = new Label("Radius: ");
            lbRadius.setAlignment(Pos.CENTER_LEFT);

            // Add TextField and corresponding Label elements to GridPane
            GridPane pane = new GridPane();
            pane.setHgap(5);
            pane.addRow(1, lbCenterX, tfCenterX);
            GridPane.setHalignment(tfCenterX, HPos.RIGHT);
            pane.addRow(2, lbCenterY, tfCenterY);
            GridPane.setHalignment(tfCenterY, HPos.RIGHT);
            pane.addRow(3, lbRadius, tfRadius);
            GridPane.setHalignment(tfRadius, HPos.RIGHT);

            // Add main label and GridPane to VBox
            this.setStyle("-fx-border-color: black");
            getChildren().addAll(lbInfo, pane);
        }
    }
}
\$\endgroup\$
7
\$\begingroup\$
  1. Constructor

    CircleInfo(String circleLabel, double centerX, double centerY, double radius)
    

    can be simplified to

    CircleInfo(String circleLabel, CircleObject obj)
    

    because the constructor can get x, y and radius from CircleObject.

  2. The same for checkIntersection(...). This method can belong to CircleObject, such that you can call it from the object itself, i.e., circle1.checkIntersection(circle2). The code is more readable, and the method is easier to extend with polymorphism to support SquareObject in the future. Property settings for CircleObject can also be simplified.

  3. paintCircleInfo() re-creates labels and edits the box every time Redraw Cirlce is clicked. This can be a performance issue, so usually these items are only created in the first pass.

  4. private static String intersectAnswer = ...; is good because you use intersectAnswer as a constant. It can be better by adding modifier final because you are not going to change it. Meanwhile, naming convention for constant in Java would capitalize letters and words are separated with under score, i.e., private static final String INTERSECT_ANWSER = ... is better.

  5. Try to remove modifier static from private static Label intersect;. You may refer to: Why are static variables considered evil?

\$\endgroup\$
  • \$\begingroup\$ Thanks, I appreciate it that was some very good advice. That was a great idea with the checkIntersection and passing the object itself \$\endgroup\$ – John Conner Jul 30 '14 at 18:16
  • \$\begingroup\$ Also I liked #4, I guess I was just trying to make it easier to create those boxes while reducing clutter in the start method, but now that you mention it I don't suppose it was a good tradeoff and probably just increased the size of my source rather then cutting back on clutter like I intended. \$\endgroup\$ – John Conner Jul 30 '14 at 18:24
6
\$\begingroup\$

Do not use underscores on Class names :

public class Project16_8 extends Application { ...

change this to

public class InterceptingCirclesApplication extends Application {

also the start() is like the main method, keep it simple.

There is too much going on in your start method.

change it to only contain 4-6 lines

    Node mainView = new MainView();
    Scene scene = new Scene(mainView);
    primaryStage.setTitle("IntersectingCircles"); 
    primaryStage.setScene(scene); 
    primaryStage.show(); 

create a MainView class to conatin the main Interface


again, from the book Clean Code

Long lists of imports are daunting to the reader. We don’t want to clutter up the tops of our modules with 80 lines of imports.

I find this to be lot of imports too. But It's still somewhat OK,

import javafx.application.Application;
import javafx.event.ActionEvent;
import javafx.event.EventHandler;
import javafx.geometry.HPos;
import javafx.geometry.Insets;
import javafx.geometry.Pos;
import javafx.scene.Scene;
import javafx.scene.control.Button;
import javafx.scene.control.Label;
import javafx.scene.control.TextField;
import javafx.scene.layout.GridPane;
import javafx.scene.layout.HBox;
import javafx.scene.layout.Pane;
import javafx.scene.layout.VBox;
import javafx.scene.paint.Color;
import javafx.scene.paint.Paint;
import javafx.scene.shape.Circle;
import javafx.stage.Stage;

Use wildcards * to reduce number of imports. If you feel the top of the class is cluttered.

@Override // Override the start method in the Application class

This falls under too much information category.

/**
 * Created by John on 7/28/2014.
 */

Use the @author java-doc annotation to specify the author.

You do not need to specify the date of creation use Revision control instead. Next thing you know is you will be writing a log up there.

\$\endgroup\$
  • 1
    \$\begingroup\$ In Java the convention is to show all imports and not use *. \$\endgroup\$ – toto2 Aug 9 '14 at 15:29
  • \$\begingroup\$ @toto2 : review process is opinion based. Updated the answer \$\endgroup\$ – bhathiya-perera Aug 9 '14 at 16:18
  • 1
    \$\begingroup\$ Thanks, for the input. I am currently working on my first big project which is going to be a poker game. I agree on the main method and I figure it would make it a bit cleaner. \$\endgroup\$ – John Conner Aug 11 '14 at 21:43
  • 1
    \$\begingroup\$ And yeah I guess I got in too much of a habit of writing that down on the Override as I was learning \$\endgroup\$ – John Conner Aug 11 '14 at 21:44

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.