Java switch case not functioning as expected

Question

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

You forgot to add break after each case in switch statement. — Prateek
– Prateek, Commented Nov 27, 2013 at 19:55
@Spud: Check out docs.oracle.com/javase/tutorial/java/nutsandbolts/switch.html — Jason C
– Jason C, Commented Nov 27, 2013 at 19:56
Thank you all. I have added the break after each case statement, but still the background color is not changing. Do I have mistakes somewhere else in the code? — Spud
– Spud, Commented Nov 27, 2013 at 20:07
@Spud Can you provide a sample input and output you are getting and what you are expecting. That would help us to pinpoint the source of the error. — Prateek
– Prateek, Commented Nov 27, 2013 at 20:15
@Prateek - My expectation is that the background color of the container will change according to the selection I make from the combobox. I have added the breaks to each case and I have removed the else statement. Regardless of the selection I make, the background color never changes. Thank you for your help — Spud
– Spud, Commented Nov 27, 2013 at 20:20

Rahul Tripathi · Accepted Answer · 2013-11-27 20:38:46Z

3

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.

edited Nov 27, 2013 at 20:38

answered Nov 27, 2013 at 19:56

Rahul Tripathi

173k33 gold badges292 silver badges341 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

Spud Over a year ago

Thank you for your response. I have taken your suggestion but still it is not changing the background color. I removed the else statement, but for each selection in the combobox, the color remains unchanged. Have I made a mistake somewhere else in the code?

Prateek Over a year ago

It won't work either @Rahul. For some reason his colors is null

Spud Over a year ago

@RahulTripathi & Prateek - Thank you both for looking into this. I still don't know why colors would be null. I would really like to understand what is going on in the code, that is why I did the experiment in the first place. Would appreciate any help.

Prateek · Accepted Answer · 2013-11-27 22:24:42Z

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);
        }

Good on ya, mate!! So often I fail to see the forest through all the trees. As soon as you pointed it out, it made perfect sense. Thanks for sticking with it. It is a lesson I will remember.Cheers

Masudul · Accepted Answer · 2013-11-27 19:57:32Z

1

You should use break after each case.

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

answered Nov 27, 2013 at 19:57

Masudul

22k5 gold badges45 silver badges60 bronze badges

Comments

WillBD · Accepted Answer · 2013-11-27 19:57:12Z

0

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);
        }

answered Nov 27, 2013 at 19:57

WillBD

1,9191 gold badge19 silver badges26 bronze badges

Comments

Uresh K · Accepted Answer · 2013-11-27 19:59:06Z

0

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.

answered Nov 27, 2013 at 19:59

Uresh K

1,13611 silver badges17 bronze badges

Comments

ac23 · Accepted Answer · 2023-06-23 14:11:56Z

0

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);

answered Jun 23, 2023 at 14:11

ac23

911 gold badge2 silver badges5 bronze badges

Collectives™ on Stack Overflow

Java switch case not functioning as expected

6 Answers 6

3 Comments

1 Comment

Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

6 Answers 6

3 Comments

1 Comment

Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related