1
\$\begingroup\$

I have this project that should continuously animating using for loop. Also I want to include variations on the position where the ball pass through. Can anyone please check my code if I am following best practices in coding. Also feel free to edit my code and suggest me some refinements. Thank you.

 package movingball;
import java.awt.Color;
import java.awt.Graphics;
import javax.swing.JFrame;
import javax.swing.JPanel;
public class MovingBall extends JPanel{
 private int ballX = 30;
 private int ballY = 30;
 private int pattern = 1;
 private int limitHeight;
 private int limitWidth;
 boolean horizontalBoundary = true;
 boolean verticalBoundary = true;
 public MovingBall(){
 setBackground(Color.BLACK);
 }
 public MovingBall(int x, int y){
 x = this.ballX;
 y = this.ballY; 
 repaint();
 }
 public static void main(String[] args) {
 JFrame frame = new JFrame();
 frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
 frame.setSize(500,700);
 MovingBall movingBall = new MovingBall();
 frame.add(movingBall); 
 frame.setVisible(true);
 BallUsingThread ball = new BallUsingThread(movingBall);
 Thread first = new Thread(ball);
 first.start();
 }
 @Override
 public void paintComponent(Graphics canvas){
 super.paintComponent(canvas);
 canvas.setColor(Color.BLUE);
 canvas.fillOval(ballX, ballY, 100, 100);
 }
 public void animateBall(){
 if(horizontalBoundary && verticalBoundary){
 if(pattern == 1){
 diagonalDown(getWidth()-100,365); 
 }else if(pattern == 2){
 diagonalDown(getWidth(),150); 
 }
 }
 if(!horizontalBoundary && !verticalBoundary){
 pattern = 3;
 if(pattern == 3){
 diagonalDownLeft(150, getHeight());
 pattern = 4;
 } 
 } 
 if(horizontalBoundary && !verticalBoundary){
 if(pattern == 4){
 diagonalUp(0, 490);
 } 
 if(pattern == 5){
 diagonalUp(300,0);
 }
 }
 if(!horizontalBoundary && verticalBoundary){
 diagonalUpRight(getWidth(),50);
 pattern = 5;
 }
 repaint();
 }
 public void diagonalDown(int limitWidth, int limitHeight){
 this.limitWidth = limitWidth;
 this.limitHeight = limitHeight;
 if((ballX += 30) >= limitWidth){
 horizontalBoundary = false; 
 } 
 if((ballY += 30) >= limitHeight){
 verticalBoundary = false; 
 } 
 }
 public void diagonalUp(int limitWidth, int limitHeight){
 this.limitWidth = limitWidth;
 this.limitHeight = limitHeight;
 if((ballX -= 30) <= limitWidth) {
 horizontalBoundary = false;
 } 
 if((ballY -= 30) <= limitHeight){
 verticalBoundary = true; 
 } 
 }
 public void diagonalUpRight(int limitWidth, int limitHeight){
 this.limitWidth = limitWidth;
 this.limitHeight = limitHeight;
 if((ballX += 30) >= limitWidth) {
 horizontalBoundary = true;
 } 
 if((ballY -= 30) <= limitHeight){
 verticalBoundary = false; 
 } 
 }
 public void diagonalDownLeft(int limitWidth, int limitHeight){
 this.limitWidth = limitWidth;
 this.limitHeight = limitHeight;
 if((ballX -= 30) <= limitWidth){
 horizontalBoundary = true;
 }
 if((ballY += 30) >= limitHeight){
 verticalBoundary = false;
 }
 //System.out.print("downleft");
 }
}
` 
 class BallUsingThread implements Runnable{
 private final MovingBall movingBall;
 public BallUsingThread(MovingBall mb){
 movingBall = mb;
 }
 @Override
 public void run() {
 for(;;){
 movingBall.animateBall();
 try {
 Thread.sleep(100);
 } catch (InterruptedException ex) {
 System.out.printf("Error",ex);
 }
 } 
 } 
 }
asked Feb 28, 2016 at 18:31
\$\endgroup\$
0

1 Answer 1

2
\$\begingroup\$

Take care of threading policies

When working with swing, you should follow its concurrency pattern. Failing to do this results in weird bugs, and crazy program behavior.

When starting your program, you should delegate this to the Swing Event Thread using either SwingUtilities.invokeLater or SwingUtilities.invokeAndWait, this makes sure the initial gui creation is done correctly:

public static void main(String[] args) {
 SwingUtilities.invokeLater(new Runnable(){ public void run() {
 JFrame frame = new JFrame();
 frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
 frame.setSize(500,700);
 MovingBall movingBall = new MovingBall();
 frame.add(movingBall); 
 frame.setVisible(true);
 ...
 }});
}

The same goes for the updating of the ball using your separate thread, but better solutions for this exists....

Using a dedicated timer for updating instead of rolling your own

You are using a custom made solution for updating the ball, by re-using existing solutions already in the framework, you get better timer accuracy, and fix the threading problem in a single step.

To do this, we are removing the BallUsingThread class, and then placing the update code in your using a swing Timer:

Timer timer = new Timer(100, new ActionListener() {
 public void actionPerformed(ActionEvent evt) {
 movingBall.animateBall();
 }
});
timer.start();

The first argument to the timer class is the delay, we set that to 100 milliseconds, and then comes the action listener that will be called every previous milliseconds.

Useless if statement

 pattern = 3;
 if(pattern == 3){
 diagonalDownLeft(150, getHeight());
 pattern = 4;
 }

This is either a typo in your code, or it is a if statement that is always true, if the latter replace it with:

 diagonalDownLeft(150, getHeight());
 pattern = 4;
answered Feb 28, 2016 at 19:20
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.