Just a few suggestions.
Things I would fix:
The serializable class DrawPanel
does not declare a static final serialVersionUID
field of typeThe serializable class DrawPanel
does not declare a static final serialVersionUID
field of type long
long
(thanks to @GilbertLeBlanc for the link):
You have a lot ofDon't if
statements with bracesever do this:
ifcatch (downException exc)
{
oneY++;
}
IfSay that you don't expand on themdo get an (which I don't seeException
thrown at you. You aren't handling it right now in any way. At least print a needstack trace to), help you can save some LOCsolve problems you may encounter in the future:
ifcatch (downException exc)
{
oneY++; exc.printStackTrace();
}
Don't everIt was also mentioned in the comments to catch specific Exceptions
. Here there is no real need to do thisthat since only an InterruptedException
is being thrown. When you do need to catch multiple Exceptions
, some people might tell you to do this:
catch (ExceptionIOException excio)
{
io.printStackTrace()
}
catch (InterruptedException ie)
{
ie.printStackTrace();
}
Recommendations that are optional:
Say that you do get anYou have a lot of Exceptionif
thrown at you. You aren't handling it right now in any way. At least print a stack trace to help you solve problems you may encounter in the futurestatements with braces:
catch if(Exception excdown)
{
exc.printStackTrace();oneY++;
}
If you don't expand on them (which I don't see a need to), you can save some LOC:
if (down) oneY++;
You don't have to do this, but I prefer the OS to set the locationOS to set the location:
You could also set the frame to the center of the window:
frame.setLocationRelativeTo(null);
But I never liked that too much. It makes your program seem like a pop-up, which I despise.
Final codeRight now your are doing this with your braces:
void someMethod(int someInt){
// ...
}
Since you are a beginner, I would recommend lining up your braces (I still do this, and I've been programming for a while):
void someMethod(int someInt)
{
// ...
}
You might note that this:
public static void main(String... args)
Is a bit different than your usual:
public static void main(String[] args)
They both do the same thing, but one uses variable arity parameters.
Final code:
import java.awt.BorderLayout;
import java.awt.Color;
import java.awt.Graphics;
import javax.swing.JFrame;
import javax.swing.JPanel;
final public class Test
{
JFrame frame;
DrawPanel drawPanel;
private int oneX = 7;
private int oneY = 7;
boolean up = false;
boolean down = true;
boolean left = false;
boolean right = true;
public static void main(String[]String... args)
{
new Test().go();
}
private void go()
{
frame = new JFrame("Test");
frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
drawPanel = new DrawPanel();
frame.getContentPane().add(BorderLayout.CENTER, drawPanel);
frame.setResizable(false);
frame.setSize(300, 300);
frame.setLocationByPlatform(true);
frame.setVisible(true);
moveIt();
}
class DrawPanel extends JPanel
{
private static final long serialVersionUID = 1L;
public void paintComponent(Graphics g)
{
g.setColor(Color.BLUE);
g.fillRect(0, 0, this.getWidth(), this.getHeight());
g.setColor(Color.RED);
g.fillRect(3, 3, this.getWidth() - 6, this.getHeight() - 6);
g.setColor(Color.WHITE);
g.fillRect(6, 6, this.getWidth() - 12, this.getHeight() - 12);
g.setColor(Color.BLACK);
g.fillRect(oneX, oneY, 6, 6);
}
}
private void moveIt()
{
while (true)
{
if (oneX >= 283)
{
right = false;
left = true;
}
if (oneX <= 7)
{
right = true;
left = false;
}
if (oneY >= 259)
{
up = true;
down = false;
}
if (oneY <= 7)
{
up = false;
down = true;
}
if (up) oneY--;
if (down) oneY++;
if (left) oneX--;
if (right) oneX++;
try
{
Thread.sleep(10);
}
catch (Exception e)
{
e.printStackTrace();
}
frame.repaint();
}
}
}