Changing color via JRadioButtons and JButtons I've got my full program working as expected. My full program looks like this:



I'm posting a part of my code which changes the color of the circle. The user can change the color of the circle via two ways:


By selecting a JRadioButtonMenuItem from the JMenu.
By pressing one of the colors(JButtons) in the JPanel below the main panel.


Here is the relevant piece of code which works as expected:

    colorbuttons = new JButton[]{new JButton(),
                                 new JButton(),
                                 new JButton(),
                                 new JButton(),
                                 new JButton(),
                                 new JButton(),}; //Different color buttons


    colorbuttons[0].setBackground(Color.WHITE);
    colorbuttons[1].setBackground(Color.GREEN);
    colorbuttons[2].setBackground(Color.RED);
    colorbuttons[3].setBackground(Color.YELLOW);
    colorbuttons[4].setBackground(Color.BLUE);
    colorbuttons[5].setBackground(Color.BLACK); //Setting each color

    for(int i=0; i&lt;colorbuttons.length; i++)
    {
        colorbuttons[i].addActionListener(this);
        colorPan.add(colorbuttons[i]); //Adding into a JPanel
    }

    bar = new JMenuBar();
    menu1 = new JMenu("Edit circle");
    sub1 = new JMenu("Colour");
    colors = new JRadioButtonMenuItem[]{new JRadioButtonMenuItem("White"),
                                        new JRadioButtonMenuItem("Green"),
                                        new JRadioButtonMenuItem("Red"),
                                        new JRadioButtonMenuItem("Yellow"),
                                        new JRadioButtonMenuItem("Blue"),
                                        new JRadioButtonMenuItem("Black",true)};

    ButtonGroup group = new ButtonGroup();
    for(int i=0; i&lt;colors.length; i++)
    {
        group.add(colors[i]);
        sub1.add(colors[i]);
    }
    menu1.add(sub1);
    bar.add(menu1);

    menu1.setMnemonic(KeyEvent.VK_E);
    sub1.setMnemonic(KeyEvent.VK_C);

    ActionListener radiolisten = new ActionListener(){
        public void actionPerformed(ActionEvent e)
        {
            int getIndex = 0;
            JRadioButtonMenuItem temp = (JRadioButtonMenuItem) e.getSource();

            for(int i=0; i&lt;colors.length; i++)
            {
                if(temp == colors[i])
                    break;
                getIndex++;
            }

            color = colorbuttons[getIndex].getBackground();
            repaint();
        }
    };

    for(int i=0; i&lt;colors.length; i++)
        colors[i].addActionListener(radiolisten);


and the actionPerformed method of the class( which the JButtons are added to):

public void actionPerformed(ActionEvent e)
{
    JButton temp = (JButton)e.getSource();
    color = temp.getBackground();
    int getIndex=0;

    for(int i=0; i&lt;colorbuttons.length; i++)
    {
        if(temp == colorbuttons[i])
            break;
        getIndex++;
    }

    colors[getIndex].setSelected(true);
    repaint();
}


color is a global variable that denotes the color of the circle.

I would like to shorten the above code snippet, especially both the actionPerformed methods. How can I do it?
For my first code review, I hope you find this useful!

Firstly, I like the fact you've used arrays for your buttons, this is good. The name of the array is also nice and descriptive.

colorbuttons = new JButton[]{new JButton(),


Looking at some of your other variable names, I'm not quite as happy.

bar = new JMenuBar();
menu1 = new JMenu("Edit circle");
sub1 = new JMenu("Colour");
colors = new JRadioButtonMenuItem[]{new JRadioButtonMenuItem("White"),
color 


Firstly colors I'd expect a list/array of colours - but no, it's an array of radio button menu items. I think color is your worst offender here. By reading the variable name, we don't know what object it's the color of.



Now, you have buttons related to a subset of colours. So I would start with an enum that will keep a track of the Color and the name

private enum EColorSet {
    eWhite(Color.WHITE, "White"),
    eGreen(Color.GREEN, "Green"),
    eRed(Color.RED, "Red"),
    eYellow(Color.YELLOW, "Yellow"),
    eBlue(Color.BLUE, "Blue"),
    eBlack(Color.BLACK, "Black");

    Color c;
    String label; 

    private EColorSet(Color c, String label)
    { 
        this.c = c;
        this.label = label;
    }
}


Now that you have a set of colours to iterate against, you can set up your arrays as follows

colorbuttons = new JButton[EColorSet.values().length]
colors = new JRadioButtonMenuItem[EColorSet.values().length]


and initialise them whilst looping through the colours

for (EColorSet currentColor : EColorSet.values()) {
    colourbuttons[currentColor.ordinal()] = new JButton();
    colourbuttons[currentColor.ordinal()].setBackground(currentColor.c);
    //do the same for the colors menu items -note you use the label...
}


see the fact in colourbuttons[currentColor.ordinal()] .setBackground(currentColor.c); i'm using the colour stored in my enum

We need to map the button (or menu item click) to an index in the array, which also represents a EColorSet instance. Unfortunately, we still need the index of the button in the array, but 

public void actionPerformed(ActionEvent e) {
    JButton temp = (JButton)e.getSource();

    int index = 0;
    for(JButton b : colourbuttons) {
        if(temp.equals(b)) {
            break;
        }
        index ++;
    }
    EColorSet currentColor = EColorSet.values()[index];
    setColor(currentColor);
    setMenuFromColor(currentColor);
}


Now to finish this off, when a button is clicked, we need to find the index, but we can do this as we know the color now.

private void setMenuFromColor(EColorSet colourClicked){
    colors[colorClicked.ordinal].setSelected(true);
    //set the menu to selected
}



private void setColor(EColorSet colourClicked){
    //set the colour of the circle
}