Einfache Java-Animation mit Swing

78057
Jeremy Johnson

Ich lerne Java Swing zu verwenden und habe eine einfache Animation erstellt, die eine kleine Form an den vorgegebenen Rändern eines Bedienfelds abhebt. Das Programm funktioniert gut, aber ich suche nach Rückmeldungen in Bezug auf Qualität und Verbesserungen oder alternative Methoden, die in einer solchen Anwendung verwendet werden könnten.

import javax.swing.*;
import java.awt.*;

final public class Tester {

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

    private void go() {
        frame = new JFrame("Test");
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

        drawPanel = new DrawPanel();

        frame.getContentPane().add(BorderLayout.CENTER, drawPanel);

        frame.setVisible(true);
        frame.setResizable(false);
        frame.setSize(300, 300);
        frame.setLocation(375, 55);
        moveIt();
    }

    class DrawPanel extends JPanel {
        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 exc){}
            frame.repaint();
        }
    }
}
Antworten
23
import.java.awt.BorderLayout sollte effizienter sein vor 5 Jahren 0
Versuchen Sie es mit einem Schalter anstelle von if / else sta vor 3 Jahren 0

5 Antworten auf die Frage

26
syb0rg

Things I would fix:

Don't do this:

import javax.swing.*;
import java.awt.*;

It could be problematic for the compiler to import a bunch of packages at once. If two packages provide the same type, both are imported, and the type is used in the class, a compile-time error occurs. This is described in JLS 6.5.5.1:

Otherwise, if a type of that name is declared by more than one type-import-on-demand declaration of the compilation unit, then the name is ambiguous as a type name; a compile-time error occurs.

In addition, it also saves a bit of memory. And your IDE (if you use one), should have the ability to do this automatically.


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

private static final long serialVersionUID = 1L;

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

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.

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++;

Right now you are setting the location:

frame.setLocation(375, 55);

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

frame.setLocationByPlatform(true);

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.


I always do frame.setVisible(true) after I am completely finished setting up the frame.


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)
{
    // ...
}

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


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();
        }
    }
}
Sehr hilfreiche Einblicke! Danke, dass Sie so viel in die Antwort gesteckt haben. Ich wusste gar nicht, wie man einen Standort auf Betriebssystembasis setzt, und ich werde es versuchen, um zu sehen, wie es mir gefällt. Es scheint in vielerlei Hinsicht vernünftiger zu sein, wenn Menschen unterschiedliche Bildschirmauflösungen haben. Meine einzige Frage ist, was funktioniert: private static final long serialVersionUID = 1L; wirklich tun Jeremy Johnson vor 6 Jahren 0
Ich habe gerade versucht, den Ort basierend auf dem Betriebssystem einzustellen, und ich mag, dass es VIEL besser ist! Vielen Dank, dass Sie mir das gezeigt haben! Jeremy Johnson vor 6 Jahren 0
@ Jeremy Johnson: serialVersionUID; http://stackoverflow.com/questions/285793/what-is-a-serialversionuid-and-why-should-i-use-it Gilbert Le Blanc vor 6 Jahren 2
@ GilbertLeBlanc, wenn mir der Punkt hier fehlt, aber bedeutet das nicht, dass die serialVersionUID nicht in einer Klasse angegeben wird, in der Serializable nicht implementiert ist? Jeremy Johnson vor 6 Jahren 0
@ Jeremy Johnson: Eine serialVersionID ist nicht erforderlich, es sei denn, die Klasse oder eine Klasse, die Sie erweitern, implementiert Serializable. Gilbert Le Blanc vor 6 Jahren 0
Das ist eine gute Information in der Hüfttasche. Vielen Dank für die Klarstellung. Ich bin wirklich dankbar für all die Hilfe, die ich bekommen kann. Jeremy Johnson vor 6 Jahren 0
@JeremyJohnson Ich habe meine Antwort ein wenig aktualisiert, seit Sie Anfänger sind. Ich habe meine Antwort nun in zwei Abschnitte aufgeteilt, eine davon würde ich definitiv korrigieren, eine in Empfehlungen, die optional sind. Ich habe auch einige zusätzliche Anmerkungen hinzugefügt. Alle Notizen wurden in meinen Code eingefügt. syb0rg vor 6 Jahren 0
@ syb0rg Er setzt die Klammern nach "Java" (Oracle) -Standard ein, daran ist nichts auszusetzen. Das ist Geschmackssache und IMHO hat nichts mit Anfänger zu tun. Das solltest du noch einmal erwähnen, IMHO. Es ist keine große Sache, wäre aber gut zu erwähnen. Marc-Andre vor 6 Jahren 0
@ Marc-Andre Deshalb ist es im Abschnitt "Empfehlungen, die optional sind". Es liegt an ihm, zu entscheiden, aber in meinen Augen erleichtert es das Programmieren. Sie haben recht, dass daran nichts falsch ist. Ich habe die Frage bearbeitet, um festzustellen, dass es eine Geschmackssache ist. syb0rg vor 6 Jahren 0
Ich schätze alle und alle Einsichten. Ich glaube nicht, dass es so etwas wie nutzlose Informationen gibt. Einige sind möglicherweise nützlicher oder korrekter als andere oder passen unter verschiedenen Umständen besser zu den Bedürfnissen. Es ist jedoch immer gut, wenn Sie für alle Fälle einen zusätzlichen Weg im Hinterkopf behalten. Wer weiß, einer meiner Professoren in diesem Jahr könnte sagen, dass es so oder so zu tun hat. Es ist gut, flexibel zu sein. Jeremy Johnson vor 6 Jahren 0
Korrigieren Sie mich, wenn ich hier falsch liege, aber nachdem ich einige weitere Java-Dokumente gelesen habe, bin ich zu ungefähr 101% sicher, dass es nicht erforderlich ist, eine serialVersionUID in diese Antwort einzuschließen (es sei denn, die Klasse sollte eine ändern, die Serializable implementiert). da die Nummer nur eine Referenz für die Deserialisierung ist. Jeremy Johnson vor 6 Jahren 0
Dies ist nur wichtig, wenn Sie Objekte direkt mit Serialisierung speichern und abrufen möchten. Es ist immer eine gute Idee, es aufzunehmen. syb0rg vor 6 Jahren 0
Normalerweise erfordert "InterruptedException" eine spezielle Behandlung. Sie sollten es nicht in einen Mehrfachfang setzen. https://www.ibm.com/developerworks/library/j-jtp05236/ Dávid Horváth vor einem Jahr 0
6
unholysampler

Your code has a lot of magic numbers in it.

if(oneX >= 283){
   right = false;
   left = true;
}

What is special about 283? What does it actually mean? If you define a constant that happens to have the value of 283, it will be much easier to update your code if the value needs to change.


try{
    Thread.sleep(10);
} catch (Exception exc){}

Catching Exception is bad. You always want to catch the most specific exception you can to do what you need. In this case, InterruptedException would be the correct exception to catch. This is not a great example because there is only one exception that can be thrown. However, in the future, it will prevent you for accidentally handling and unexpected exception the wrong way.

Additionally, you are not doing anything with the exception. In general, this is a bad practice. Exceptions are thrown for a reason and should not be ignored without some thought. If you know you don't care what the exception is cause by, add a comment to explain your reasoning to anyone who might see the code later.

Ich habe 283 verwendet, weil das Fenster nicht in der Größe verändert werden kann und die Umrisse die Form am besten mit dieser Zahl treffen. Ich begann mit verschiedenen Zahlen und änderte sie so lange, bis sie genauer zu sein schien. Ich mag die Idee, etwas zu definieren: private int rightBorder = 283. Wußte das nicht über Ausnahmen, danke! Jeremy Johnson vor 6 Jahren 1
5
Anirban Nag 'tintinmj'

You get a good review from @syb0rg. I'm going to tell you something that will help you in the long run. If you create any application in Swing in future try to use SwingUtilities.invokeLater() to prevent any unnatural behavior. So the code will be like this

public static void main(String[] args) 
{
    SwingUtilities.invokeLater(new Runnable() 
    {
        public void run() 
        {
            new Tester().go();
        }
    });
}

Now you will ask "Why should I use this?". Here is a good answer from StackOverflow.

Ich lerne heute etwas Neues, +1. syb0rg vor 6 Jahren 2
Ich kann mir vorstellen, dass dies besonders in größeren GUIs nützlich ist, ja? Jeremy Johnson vor 6 Jahren 0
@JeremyJohnson ist größer als auch für kleinere Anlässe und Sie können sich nicht auf Ihre Swing-App verlassen, da alle Komponenten nicht threadsicher sind. Warum also * die Chance ergreifen *? Nutzen Sie die API oder Sie können Ihre eigene Threadsicherheit von Grund auf implementieren, aber ich würde nicht dorthin gehen. Anirban Nag 'tintinmj' vor 6 Jahren 0
@tintinmj Ich verwende diese Animations-GUIs jetzt normalerweise in einem eigenen Thread, wenn ich an einem Test arbeite, der auch andere Operationen beinhaltet. Besonders, wenn eine der anderen Operationen einwandfrei ist. Jeremy Johnson vor 6 Jahren 0
4
Rick9399

Anstatt die ganze Richtung booleans, up, left, rightund downkönnen Sie verwenden, velocityXund velocityY(was ich rufe xVund yVvon jetzt an). Dadurch wird die Anzahl der Zeilen um einiges reduziert, da Sie nicht nur 4, sondern nur 2 Variablen benötigen. Sie müssen nur die vertikale und horizontale Kollision prüfen und die Geschwindigkeit beim Aufprall invertieren.

Setze zu Beginn xVund yVauf 1 und platziere den Ball irgendwo auf dem Feld:

if (x < 0 || x >= width) xV *= -1;

Gleiches gilt natürlich für die y-Werte. Dann in der Update-Methode:

x += xV; y += yV;

Wenn der Ball die rechte Wand berührt, ändert sich die horizontale Geschwindigkeit in -1, was bedeutet, dass x jedes Mal abnimmt.

Sie können auch die Startwerte auf 2 oder 3 ändern, um den Ball schneller zu machen.

Außerdem ist Javas Fensterbreite 6px kleiner als der Bildschirm und die Höhe 29px. Sie hatten also die 283 (wenn die Höhe tatsächlich 256px ist). Rick9399 vor 4 Jahren 0
3
Walter Laan

Sie sollten nicht auf dem Event Dispatch Thread (EDT) schlafen, da dies Swing für die Verarbeitung von Ereignissen einschließlich Malereignissen blockiert.

Sie sollten einen javax.swing.Timer anstelle einer while-Schleife verwenden:

Timer timer = new Timer(10, new ActionListener() {
        @Override
        public void actionPerformed(ActionEvent e) {
           // update variables
           // do not sleep
           // only need to repaint the panel, not entire frame
           drawPanel.repaint();
        }
      });
timer.start();

Weitere Hinweise:

  • Wenn Sie EXIT_ON_CLOSE nicht verwenden, sollten Sie einen WindowListener hinzufügen, um den Timer zu stoppen, wenn das Fenster geschlossen wird
  • Sie können nur den Begrenzungsrahmen des alten und des neuen Speicherorts mit „Repaint“ (x, y, Breite, Höhe) für die Performance neu streichen
  • Siehe auch https://docs.oracle.com/javase/tutorial/uiswing/painting/index.html