Verzwicktes Problemchen mit Panel-Array

Status
Nicht offen für weitere Antworten.

Developer_X

Top Contributor
Hi, kann mir einer helfen?

Java:
	public void prepare()
	{
		setLayout(new GridLayout(y,x));

		while(a<y)
		{
			while(b<x)
			{
				colors[a][b] = new Color(255,255,255);
				
				panels[a][b] = new Panel();				
				panels[a][b].setBackground(colors[a][b]);
				panels[a][b].addMouseListener(new MouseListener()
				{
					public void mouseClicked(MouseEvent arg0)
					{
						if(arg0.getButton()==MouseEvent.BUTTON1)
						{
							setBackground(foreground);
							repaint();
						}
						if(arg0.getButton()==MouseEvent.BUTTON3)
						{
							setBackground(background);
							repaint();
						}
					}
					public void mouseEntered(MouseEvent arg0)
					{
					}
					public void mouseExited(MouseEvent arg0) 
					{
					}
					public void mousePressed(MouseEvent arg0)
					{
					}
					public void mouseReleased(MouseEvent arg0) 
					{
					}
				});
				add(panels[a][b]);
				b++;
			}
			b=0;
			a++;
		}
		a = 0;
		b = 0;
	}

Mein Ziel ist folgendes:
Wenn man mit eienr Maustaste auf einen der Panels drückt, dann soll dieser, der gedrückte, entweder in die Color "foreground" oder "background" zu färben, kann mir bitte einer helfen?

Danke,
Developer_X
 
ich machs nicht ganz einfach mit
Java:
                panels[a][b].setBackground(colors[a][b]);
weil die werte a und b nie wieder verwendet werden, und die methode einmal verwendet wird.
Das heißt, würde man, egal auf welches Panel, klicken, immer nur das erste oben, färben, also panel[0][0]
da am ende der methode die beiden werte auf 0 gesetzt werden. was soll ich also tun?
 
Ohne mir jetzt den Code genauer angeschaut zu haben oder länger darüber nachgedacht zu haben was Du eigentlich willst.

Ich glaube her ausgelesen zu haben, dass Du mehrere Panels hast. Diese sollen die Hintergrundfarbe, so wie die Vordergrundfarbe ändern können, je nach dem ob sie den Focus haben/selektiert wurden oder nicht. Soweit richtig?

Wenn ja, dann wäre es nicht falsch in einem, wie war das noch, KKBS zu zeigen was nicht funktioniert, bzw. anhand eines Bildes was Du haben möchtest.

Egal ob ich nun richtig liege oder nicht, glaube ich jede andere Lösung als Deine wäre besser geeignet das Problem zu lösen ( Subjektiv anhand des wirklich nur überflogenen Quelltextes )

Also bitte klarer machen was Du möchtest oder was nicht so funktioniert wie es soll.
 
was soll ich also tun?

in den Listenern keine Index-Werte verwenden, die später zur Laufzeit auf 0 gesetzt werden. Das kannst du auch gar nicht 😀 Weil anonyme Klassen genau wegen der Vemeidung solcher fataler Situationen finale Variablen erwarten. D.h. du musst dir innerhalb der Listener zwei finale Kopien von a und b erzeugen, und dann wie gehabt

Java:
panels[finalA][finalB].setBackground(...);

Dass dein Code überhaupt kompiliert liegt daran, dass die Klasse in der du arbeitest wohl auch eine (J)Component ist, und daher die Methoden setBackground/foreground bereitstellt. Und genau darauf bezieht sich dein Code im Moment auch.

Wichtig: Nur weil ein Listener zu einem Objekt geaddet ist, heisst das doch nicht, dass sich jeglicher Code in seinen Methoden auf dieses Objekt bezieht :exclaim: Das mag vllt so rüberkommen bei

Java:
class MyPanel implements ActionListener {}

aber das ist nicht so. in diesem Fall beziehst du dich eben auf die Klasse selbst, nicht auf irgendein Panel.

Btw: repaint() ist nicht nötig, der macht das automatisch nach setBackground/setForeground

Und noch zur Information: Es gibt für (fast?) jeden Listener auch einen entsprechenden Adapter, der standardmässig alle Methoden pseudo-implementiert (die Rümpfe sind leer). Dann überschreibst du nur die Methoden, die du brauchst. Das bläht den Code bei Benutzung von anonymen Klassen nicht so auf:

Java:
panels[a][b].addMouseListener(new MouseAdapter(){
    @Override
     public void mouseClicked(MouseEvent e){
             ...
     }

    // die anderen Methoden sind implementiert (machen nichts), 
    // wenn wir sie nicht brauchen überschreiben wir sie einfach nicht.
});

Und da ich grad on fire bin hör ich noch immer nicht auf 😉

1) Du verwendest wie ich finde noch immer zu viele globale Variablen. Warum genau sind a, b und x,y nicht lokale Variablen der Methode prepare()? Es ist ziemlich fehleranfällig Variablen durch mehrere Methoden und Objekte zu ziehen, weil sie überall verändert werden können und du nie genau sagen kannst, was jetzt welche Variable für einen Wert hat. Wenn es also keinen Grund dafür gibt, warum sie nicht lokal sein dürfen (und den gibt es ganz sicherlich nicht, wenn doch, dann sag bitte Bescheid), dann änder das bitte.

Ich schätze mal du hälst das für performanter, nicht "unnötig" lokale Variablen zu erzeugen. Dann lass dir gesagt sein, dass es nicht performanter ist. Ich will hier nicht über nano-sekunden reden. Dein Stack kann mehr lokale Variablen speichern als du Sekunden in deinem Leben lebst, und bei Objekten zerstört der GC (Garbage Collector) auch sofort alles, was nicht mehr benötigt wird.
@mods: GC sollte nen Tool Tip bekommen

2) Was du allerdings besser machen kannst:
Java:
colors[a][b] = new Color(255,255,255);
Du erstellst bei jedem Durchlauf ein neues Objekt mit der gleichen Farbe, ganz abgesehen davon ist das auch noch eine vorgefertigte Konstante (Color.WHITE)

3) arg0 ist hässlich, das solltest du ersetzen. Okay, "e" ist vllt auch nicht das Wahre, aber immerhin deutet es ein Event an,und arg0 kann alles sein. Ist einfach eine dieser Konventionen, die du ja so sehr hasst und wahrscheinlich auch völlig ignorieren willst.

4) Um auch mal was Positives zu sagen: Abgesehen von den globalen Variablen sind das, was ich bemängel, ja alles eher unwichtige Kleinigkeiten (was nicht heißt dass du sie ignorieren sollst!). Alles in allem könnte die prepare()-Methode tatsächlich von jemandem geschrieben sein, der Ahnung von Java hat bzw. der verständlichen Quellcode schreibt 😉 Zumindest schon um einiges besser als was ich bisher von dir gesehen habe.
 
Zuletzt bearbeitet:
in Zeile 4 würde ich genau eine einzige finale Instanz eines MouseAdapters deklarieren und initialisieren (natürlich über mehrere Zeilen verteilt). Innerhalb des MouseAdapter dürfte dann ((Component) arg0.getSource()).setBackground()" bzw. "arg0.getComponent().setBackground()" das tun was du haben willst.
Java:
    public void prepare()
    {
        setLayout(new GridLayout(y,x));
        final MouseAdapter listener = new MouseAdapter() {
            @Override
            public void mouseClicked(MouseEvent me) {
                Component source = me.getComponent();
                int button = me.getbutton();
                if(button == MouseEvent.BUTTON1) {
                    source.setBackground(background);
                }
                if(button == MouseEvent.BUTTON3) {
                    source.setBackground(foreground);
                }
            }
        }
        while(a<y)
        {
            while(b<x)
            {
                colors[a][b] = new Color(255,255,255);
                
                panels[a][b] = new Panel();             
                panels[a][b].setBackground(colors[a][b]);
                panels[a][b].addMouseListener(listener);
                add(panels[a][b]);
                b++;
            }
            b=0;
            a++;
        }
        a = 0;
        b = 0;
    }
 
Status
Nicht offen für weitere Antworten.

Neue Themen


Zurück
Oben