CODEWITHSUNDEEP
javaswitch-statement
Spud
Spud

Reputation: 371

Java switch case not functioning as expected

I am a student of very basic Java. We did an assignment to make the background color change according to the radio button selected using several if statements. That worked fine. I decided to change the selection process to a combobox and use a switch case. It seems to me the process is failing the if statement in the switch case method. I'm trying to get a better understanding of how things work. The code:

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

class Lab17_4combo extends JFrame implements ActionListener
{
    Container container;
    JComboBox colors;

    public Lab17_4combo()
    {
        super("ComboBox ");
        container = this.getContentPane();
        container.setLayout(new FlowLayout());
        setSize(300,200);
        setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

        String[] selectColor = {"Red", "Yellow", "Blue", "Green", "Magenta"};
        JComboBox colors = new JComboBox(selectColor);
        colors.setSelectedIndex(-1);
        colors.addActionListener(this);

        container.add(colors);
        setVisible(true);
    }
    public void actionPerformed(ActionEvent e)
    {
        int chgColor;

        if(e.getSource() == colors)
        {
            chgColor = colors.getSelectedIndex();

            switch(chgColor)
            {
                case 0:
                    container.setBackground(Color.red);
                case 1:
                    container.setBackground(Color.yellow);
                case 2:
                    container.setBackground(Color.blue);
                case 3:
                    container.setBackground(Color.green);
                case 4:
                    container.setBackground(Color.magenta);
            }
        }else
            {
                container.setBackground(Color.magenta);
            }

    }
    public static void main(String[] args)
    {
        Lab17_4combo s = new Lab17_4combo();
    }
}

I put in the else to check if it was failing the if. I'm assuming that is where the problem is, but I don't know how to fix it. Any help would be greatly appreciated. The original assignment has been completed, this is my own experimentation. I'm not asking for anyone to do my homework for me. Cheers

EDIT-- I have made the suggested changes to the code (Thanks to all for the suggestions). The background color of the container still does not change regardless of the selection I make from the combobox. I'm assuming there are mistakes elsewhere in the code, but I'm at a loss to find them. My expectation is that the background color of the container will change according to the selection I make from the combobox. This is not happening.

The revised code:

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

class Lab17_4combo extends JFrame implements ActionListener
{
    Container container;
    JComboBox colors;

    public Lab17_4combo()
    {
        super("ComboBox ");
        container = this.getContentPane();
        container.setLayout(new FlowLayout());
        setSize(300,200);
        setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

        String[] selectColor = {"Red", "Yellow", "Blue", "Green", "Magenta"};
        JComboBox colors = new JComboBox(selectColor);
        colors.setSelectedIndex(-1);
        colors.addActionListener(this);

        container.add(colors);
        setVisible(true);
    }
    public void actionPerformed(ActionEvent e)
    {
        int chgColor;

        if(e.getSource() == colors)
        {
            chgColor = colors.getSelectedIndex();

            switch(chgColor)
            {
                case 0:
                    container.setBackground(Color.red);
                    break;
                case 1:
                    container.setBackground(Color.yellow);
                    break;
                case 2:
                    container.setBackground(Color.blue);
                    break;
                case 3:
                    container.setBackground(Color.green);
                    break;
                case 4:
                    container.setBackground(Color.magenta);
                    break;
            }
        }
    }
    public static void main(String[] args)
    {
        Lab17_4combo s = new Lab17_4combo();
    }
}

With my limited knowledge of Java I'm not able to see where the mistake(s) may be. Any help would be appreciated.Cheers

Upvotes: 3

Views: 2450

Answers (6)

ac23
ac23

Reputation: 91

I think you are using the wrong colors object inside the constructor, instead of

JComboBox colors = new JComboBox(selectColor);

you have to use the colors attribute

colors = new JComboBox(selectColor);

Upvotes: 0

Prateek
Prateek

Reputation: 1926

As mentioned in my comment you forgot to add break in each of your case.

           switch(chgColor)
            {
                case 0:
                    container.setBackground(Color.red);
                    break;
                case 1:
                    container.setBackground(Color.yellow);
                     break;
                case 2:
                    container.setBackground(Color.blue);
                    break;
                case 3:
                    container.setBackground(Color.green);
                    break;
                case 4:
                    container.setBackground(Color.magenta);
                    break;
                default:
                //What if none of above condition is satisfied.
             }

Edit: - Check the comments in the code

    class Lab17_4combo extends JFrame implements ActionListener
    {
        Container container;
        JComboBox colors;// you declared colors. 

        public Lab17_4combo()
        {
            super("ComboBox ");
            container = this.getContentPane();
            container.setLayout(new FlowLayout());
            setSize(300,200);
            setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

            String[] selectColor = {"Red", "Yellow", "Blue", "Green", "Magenta"};

            //JComboBox colors = new JComboBox(selectColor);// You declared colors again. Change     this line to 

            colors = new JComboBox(selectColor);
            colors.setSelectedIndex(-1);
            colors.addActionListener(this);

            container.add(colors);
            setVisible(true);
        }

Upvotes: 1

Rahul Tripathi
Rahul Tripathi

Reputation: 172608

You forgot the break statement after each case

Try this:

switch(chgColor)
            {
                case 0:
                    container.setBackground(Color.red);
                     break;
                case 1:
                    container.setBackground(Color.yellow);
                    break;
                case 2:
                    container.setBackground(Color.blue);
                     break;
                case 3:
                    container.setBackground(Color.green);
                    break;
                case 4:
                    container.setBackground(Color.magenta);
                    break;
                default:
                //You may add a default case here.
            }

EDIT:-

I think your condition 

if(e.getSource() == colors)

is never true thats why you are getting this problem. You may try to compare like this:

if(e.getSource().equals(colors))

Always use .equals method when you are comparing objects.

Upvotes: 3

Uresh K
Uresh K

Reputation: 1136

Looks like you always end up with magenta color given the if condition and the switch statement.

Use break for each case statement.

In the switch statement, for each case, you do not have a break;. So even if switch finds a valid matching case, it executes that case and, in your code (since you do not break the control to come out of that case), it eventually passes the control to the cases following the matching case and always ending up in the last case.

Upvotes: 0

Masudul
Masudul

Reputation: 21981

You should use break after each case.

       case 0:
          container.setBackground(Color.red);
          break;
        case 1:
          container.setBackground(Color.yellow);
          break;
          ....

Upvotes: 1

WillBD
WillBD

Reputation: 1919

You need to make sure and add a break; after every line in the case, like this: 

        switch(chgColor)
        {
            case 0:
                container.setBackground(Color.red);
                break;
            case 1:
                container.setBackground(Color.yellow);
                break;
            case 2:
                container.setBackground(Color.blue);
                break;
            case 3:
                container.setBackground(Color.green);
                break;
            case 4:
                container.setBackground(Color.magenta);
                break;
        }
    }else
        {
            container.setBackground(Color.magenta);
        }

Upvotes: 0

Related Questions