Skip to main content
replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

The serializable class DrawPanel does not declare a static final serialVersionUID field of type longThe serializable class DrawPanel does not declare a static final serialVersionUID field of type long (thanks to @GilbertLeBlanc for the link):

It was also mentioned in the comments to catch specific Exceptions. Here there is no real need to do that since only an InterruptedException is being thrown. When you do need to catch multiple Exceptions, some people might tell you to do thissome people might tell you to do this:

They both do the same thing, but one uses variable arity parametersvariable arity parameters.

The serializable class DrawPanel does not declare a static final serialVersionUID field of type long (thanks to @GilbertLeBlanc for the link):

It was also mentioned in the comments to catch specific Exceptions. Here there is no real need to do that since only an InterruptedException is being thrown. When you do need to catch multiple Exceptions, some people might tell you to do this:

They both do the same thing, but one uses variable arity parameters.

The serializable class DrawPanel does not declare a static final serialVersionUID field of type long (thanks to @GilbertLeBlanc for the link):

It was also mentioned in the comments to catch specific Exceptions. Here there is no real need to do that since only an InterruptedException is being thrown. When you do need to catch multiple Exceptions, some people might tell you to do this:

They both do the same thing, but one uses variable arity parameters.

added 597 characters in body
Source Link
syb0rg
  • 21.8k
  • 10
  • 112
  • 191

But you could also do this:

catch (IOException | InterruptedException e)
{
    e.printStackTrace();
}

Right now there are some hard-coded values:

if(oneX >= 283)
if(oneX <= 7)
...

I would store those values in variables, and then I would make it so that it is more scalable. For example:

int xScale = 280;
int yScale = xScale / 40;

That way, you only have to change one value in order to scale your different values. This is just an example, and has not been implemented in my code. I left this for you to do on your own.

This is a matter of taste, and it is perfectly fine to stay with what you are doing right now.

But you could also do this:

catch (IOException | InterruptedException e)
{
    e.printStackTrace();
}

Right now there are some hard-coded values:

if(oneX >= 283)
if(oneX <= 7)
...

I would store those values in variables, and then I would make it so that it is more scalable. For example:

int xScale = 280;
int yScale = xScale / 40;

That way, you only have to change one value in order to scale your different values. This is just an example, and has not been implemented in my code. I left this for you to do on your own.

This is a matter of taste, and it is perfectly fine to stay with what you are doing right now.

added 2059 characters in body
Source Link
syb0rg
  • 21.8k
  • 10
  • 112
  • 191

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();
        }
    }
}

Just a few suggestions.

The serializable class DrawPanel does not declare a static final serialVersionUID field of type long:

You have a lot of if statements with braces:

if(down)
{
    oneY++;
}

If you don't expand on them (which I don't see a need to), you can save some LOC:

if (down) oneY++;

 

Don't ever do this:

catch (Exception exc)
{
}

Say that you do get an Exception 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 future:

catch (Exception exc)
{
    exc.printStackTrace();
}

You don't have to do this, but I prefer the OS to set the location:

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[] 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();
        }
    }
}

Things I would fix:

The serializable class DrawPanel does not declare a static final serialVersionUID field of type long (thanks to @GilbertLeBlanc for the link):

Don't ever do this:

catch (Exception exc)
{
}

Say that you do get an Exception 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 future:

catch (Exception exc)
{
    exc.printStackTrace();
}

It was also mentioned in the comments to catch specific Exceptions. Here there is no real need to do that since only an InterruptedException is being thrown. When you do need to catch multiple Exceptions, some people might tell you to do this:

catch (IOException io)
{
    io.printStackTrace()
}
catch (InterruptedException ie)
{
    ie.printStackTrace();
}

Recommendations that are optional:

You have a lot of if statements with braces:

if(down)
{
    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 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.

Right 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... 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();
        }
    }
}
Source Link
syb0rg
  • 21.8k
  • 10
  • 112
  • 191
Loading