Problem with Java GUI implementation

I have a problem with the code I am currently trying to run - I am trying to make 3 buttons, put them on a GUI, and then have the first buttons colour be changed to orange, and the buttons next to that colour change to white and green. Every click thereafter will result in the colours moving one button to the right. My code thus far is as follows, it is skipping colours in places and is not behaving at all as I expected. Can anyone offer some help/guidance please ?

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

public class ButtonJava extends JButton implements ActionListener  {
  private int currentColor=-1;
  private int clicks=0;
  private static final Color[] COLORS = {
    Color.ORANGE,
    Color.WHITE,
    Color.GREEN };
  private static ButtonJava[] buttons;

  public ButtonJava( ){
    setBackground( Color.YELLOW );
    setText( "Pick ME" );
    this.addActionListener( this );
  }

  public static void main(String[] args) {
    JFrame frame = new JFrame ("JFrame");
    JPanel panel = new JPanel( );
    frame.setDefaultCloseOperation( JFrame.EXIT_ON_CLOSE);
    buttons = new ButtonJava[3];
    for(int i = 0;i<buttons.length ; i++){
      buttons[i] = new ButtonJava(); 
      panel.add(buttons[i]);
    }
    frame.getContentPane( ).add( panel );
    frame.setSize( 500, 500);
    frame.setVisible( true );
  }

  private void updateButton() {
     clicks++;
    changeColors();
//    setText( );
  }

private void changeColors( ) {
  for (int i=buttons.length-1;i>=0;i--){
    buttons[i].currentColor = nextColor(currentColor);
    buttons[i].setBackground(COLORS[buttons[i].currentColor]);
    buttons[i].setText(("# of clicks = " + buttons[i].getClicks() ) );
  }
}

private Integer getClicks() {
 return clicks;
}

开发者_JAVA百科private int nextColor( int curCol ) {
  final int colLen = COLORS.length;
  curCol--;
  curCol = (colLen + curCol % colLen) % colLen;
  return curCol;
}

private void firstClick( ActionEvent event ) {
  int curCol = 0;
  for (int i=buttons.length-1;i>=0;i--){
    if ( buttons[i] == event.getSource() ) {
      buttons[i].currentColor = curCol;
      curCol++;
      currentColor++;
    }
  }}

  @Override
  public void actionPerformed( ActionEvent event ) {
    if ( -1 == currentColor ) {
      firstClick( event );
    }
    updateButton( );   
  }


}

Thank you very much for the help :)

---

You have a couple issues with the code you posted, but they generally boil down to being clear about what is a member of the class(static) and what is a member of the instance.

For starters, you buttons array only exists inside your main method and can't be accessed by changeColors(). Along those same lines, since changeColors() is an instance method, setBackground() needs to be called directly on the button in your array. as written you are setting the color for one button 3 times.

Additionally, the logic in changeColors() is not properly rotating the currentColor index. You need to both increase the counter and ensure is wraps for the length of the color array. If the arrays are the same size, you need to make sure there is an extra addition to make the colors cycle.

private static void changeColors( ) {
  for (int i=0;i<buttons.length;i++){
    buttons[i].setBackground(COLORS[currentColor]);
    currentColor = nextColor(currentColor);
  }
  if (buttons.length == COLORS.length) {
    currentColor = nextColor(currentColor);
  }
}

private static int nextColor(int currentColor) {
  return (currentColor+1)% COLORS.length;
}

Edit for new code:

I'm not sure why you re-wrote nextColor(), as what I posted worked. But in general, I feel like you are running into issues because your code is not well partitioned for the tasks you are trying to achieve. You have code related to the specific button instance and code related to controlling all the buttons mixing together.

With the following implementation, the issue of how many times a button was clicked is clearly self-contained in the button class. Then every button press also calls the one method in the owning panel. This method knows how many buttons there are and the color of the first button. And each subsequent button will contain the next color in the list, wrapping when necessary.

public class RotateButtons extends JPanel {
  private static final Color[] COLORS = { Color.ORANGE, Color.WHITE, Color.GREEN };
  private static final int BUTTON_COUNT = 3;
  private JButton[] _buttons;
  private int _currentColor = 0;

  public static void main(String[] args)
  {
    JFrame frame = new JFrame("JFrame");
    frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
    frame.getContentPane().add(new RotateButtons());
    frame.setSize(500, 500);
    frame.setVisible(true);
  }

  public RotateButtons()
  {
    _buttons = new JButton[BUTTON_COUNT];
    for (int i = 0; i < _buttons.length; i++) {
      _buttons[i] = new CountButton();
      add(_buttons[i]);
    }
  }

  private void rotateButtons()
  {
    for (JButton button : _buttons) {
      button.setBackground(COLORS[_currentColor]);
      _currentColor = nextColor(_currentColor);
    }
    if (_buttons.length == COLORS.length) {
      _currentColor = nextColor(_currentColor);
    }
  }

  private int nextColor(int currentColor)
  {
    return (currentColor + 1) % COLORS.length;
  }

  private class CountButton extends JButton {
    private int _count = 0;

    public CountButton()
    {
      setBackground(Color.YELLOW);
      setText("Pick ME");
      addActionListener(new ActionListener() {
        @Override
        public void actionPerformed(ActionEvent arg0)
        {
          _count++;
          setText("# of clicks = " + _count);
          rotateButtons();
        }
      });
    }
  }
}

2nd Edit:

Shows just the changes to shift _currentColor by the necessary amount on the first click.

public class RotateButtons extends JPanel {
  ...
  private boolean _firstClick = true;
  ...
  private void rotateButtons(CountButton source)
  {
    if (_firstClick) {
      _firstClick = false;
      boolean foundSource = false;
      for (int i = 0; i < _buttons.length; i++) {
        if (foundSource) {
          _currentColor = nextColor(_currentColor);
        } else {
          foundSource = _buttons[i] == source;
        }
      }
    }
    ...
  }

  private class CountButton extends JButton {
    ...

    public CountButton()
    {
      ...
      addActionListener(new ActionListener() {
        @Override
        public void actionPerformed(ActionEvent arg0)
        {
          ...
          rotateButtons(CountButton.this);
        }
      });
    }
  }

---

One thing that I noticed is you are using currentColor to assign the color, but the currentColor is initialized to 0 and the only manipulation is currentColor %= 2 which does not change it.

---

If I'm understanding what you are wanting to achieve, I'm thinking to change currentColor %= 2 to ++currentColor, and setBackground(COLORS[currentColor]); to buttons[i].setBackground(COLORS[(i + currentColor) % 3]);. That way, your colours should cycle around your buttons each time one is clicked.

EDIT: it's probably also worth calling changeColors from within main to initialise your button colours. And, as @unholysampler notes, your array buttons is local to main and should (for example) be refactored as a static member variable, and have changeColors become a static method.