Set background of JPanel object a second time

Question

To help learn more about EventListeners in Java, I've created a simple program that consists of one JFrame and two JPanels and all it's supposed to do is toggle a secondary color on the Jpanel as it is clicked on.

My code changes each JPanel to the new secondary color when it gets clicked but when it gets clicked a second time it does revert back to the original color. What do I need to change to get it to work correctly? I've tried re-writing the code multiple times so I must be missing some concept of how EventListeners or JPanels work.

package com.spencerlarry;

import java.awt.Color;
import java.awt.GridLayout;
import java.awt.event.MouseEvent;
import java.awt.event.MouseListener;

import javax.swing.JFrame;
import javax.swing.JPanel;

public class View extends JFrame{

    public static final int OFF = 1;
    public static final int ON = 1;

    public static final String DARKGRAY = "#696969";
    public static final String CYAN = "#00FFFF";

    Space top;
    Space bottom;

    public View(){
        this.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        this.setTitle("Window Test");
        this.setSize(300, 300);
        this.setMinimumSize(getSize());

        this.setLayout(new GridLayout(2,1));
        add(new Space());
        add(new Space());

    }

    public class Space extends JPanel implements MouseListener{

        private String color;

        public Space(){
            setBackground(Color.decode(DARKGRAY));
            addMouseListener(this);
        }

        public String getColor(String c){
            return color;
        }

        public void setColor(){
            if(color == CYAN){
                setBackground(Color.decode(DARKGRAY));
            }
            else{
                setBackground(Color.decode(CYAN));
            }

        }

        @Override
        public void mouseClicked(MouseEvent e) {
            this.setColor();
        }

        @Override
        public void mousePressed(MouseEvent e) {}
        @Override
        public void mouseReleased(MouseEvent e) {}
        @Override
        public void mouseEntered(MouseEvent e) {}
        @Override
        public void mouseExited(MouseEvent e) {}
    }
}

score 3 · Accepted Answer · edited May 23 '17 at 12:29

First, don't compare Strings with ==. Your if statement should be:

if(color.equals(CYAN))

But this is going to throw a null pointer exception the first time, since you aren't initializing the value of color.

You aren't actually setting or changing the value of the color variable, so when you check

if(color == CYAN)

it always evaluates to false, meaning you are always calling

setBackground(Color.decode(CYAN));

when you click, no matter what.

In your Space constructor, you should initialize color to DARKGRAY (to avoid the null pointer exception), and then change setColour() to something like this:

public void setColor(){
    if(color.equals(CYAN)){
        setBackground(Color.decode(DARKGRAY));
        color = DARKGRAY;
    }
    else{
        setBackground(Color.decode(CYAN));
        color = CYAN;
    }
}

score 2 · Answer 2 · answered Nov 06 '15 at 22:32

In addition to the answer given by @gla3dr, why decode the color string every time? Just define:

public static final Color MY_DARKGRAY = new Color("#696969");
// there is already a standard Color.CYAN, so don't re-define it

then set the color in your constructor, and

public void setColor(){
    if( color == Color.CYAN )){
        setBackground( MY_DARKGRAY );
        color = MY_DARKGRAY;
    }
    else{
        setBackground(Color.CYAN);
        color = Color.CYAN;
    }
}

@spencerlarry Note, the `color` variable will have to be changed from a `String` to a `Color` if you do this. — gla3dr, Nov 06 '15 at 22:34

Set background of JPanel object a second time

2 Answers2