29

I am trying to compile following code without warnings:

    while (window.pollEvent(event))
    {
        switch (event.type) {
            case sf::Event::Closed:
                window.close(); break;
            case sf::Event::KeyPressed:
                if(event.key.code == sf::Keyboard::Escape )
                    window.close();
                if( sf::Keyboard::isKeyPressed( sf::Keyboard::Space ) )
                    particleSystem.fuel( 200/* * window.getFrameTime() */);
                if( sf::Keyboard::isKeyPressed( sf::Keyboard::A ) )
                    particleSystem.setPosition( --xpos, ypos );
                if( sf::Keyboard::isKeyPressed( sf::Keyboard::D ) )
                    particleSystem.setPosition( ++xpos, ypos );
                if( sf::Keyboard::isKeyPressed( sf::Keyboard::W ) )
                    particleSystem.setPosition( xpos, --ypos );
                if( sf::Keyboard::isKeyPressed( sf::Keyboard::S ) )
                    particleSystem.setPosition( xpos, ++ypos );
                if( sf::Keyboard::isKeyPressed( sf::Keyboard::Left ) )
                    particleSystem.setGravity( --xgrv * 0.1f, ygrv * 0.1f);
                if( sf::Keyboard::isKeyPressed( sf::Keyboard::Right ) )
                    particleSystem.setGravity( ++xgrv * 0.1f, ygrv * 0.1f );
                if( sf::Keyboard::isKeyPressed( sf::Keyboard::Up ) )
                    particleSystem.setGravity( xgrv * 0.1f, --ygrv * 0.1f );
                if( sf::Keyboard::isKeyPressed( sf::Keyboard::Down ) )
                    particleSystem.setGravity( xgrv * 0.1f, ++ygrv * 0.1f );
                if( sf::Keyboard::isKeyPressed( sf::Keyboard::G ) )
                    particleSystem.setGravity( 0.0f, 0.0f );
                if( sf::Keyboard::isKeyPressed( sf::Keyboard::P ) )
                    particleSystem.setPosition( 320.0f, 240.0f );
                break;
    }

however, I am getting a lot of warnings:

/home/bluszcz/private/repo/deerportal/game.cpp:444: warning: enumeration value 'LostFocus' not handled in switch [-Wswitch]

Which in my it is not an issue, since I am don't need handling all types of the events.

Adding

default:
    break;

to my code removes the warnings, however is it a best way to solve this issue?

bluszcz
  • 4,054
  • 4
  • 33
  • 52
  • 1
    I think `default` is still the best way, you can still handle every event separately if you want to. – Rakete1111 May 16 '16 at 13:00
  • 1
    You should almost always have a default case in case there is no match. – NathanOliver May 16 '16 at 13:03
  • I personally believe this warning (together with reordering of member initialization in constructor) is one of the least useful in gcc. I had tons of cases where not all enum values had a meaningful case switches, and doing an empty default just to pacify the compiler is meaningless syntax noise. That's why this warning is the one I am OK with suppressing. – SergeyA May 16 '16 at 13:17
  • 1
    @NathanOliver I disagree. Sure, there are cases where a `default:` case is appropriate. But in the general case you should avoid it and handle all values explicitly so that the compiler can warn you when you forgot to handle a case. – Jesper Juhl May 16 '16 at 14:20

1 Answers1

53

Be explicit

It depends on what you are trying to achieve. The governing rule is

It is better to be explicit.

Omitting the cases simply makes it look like you forgot some. Being explicit assures subsequent readers of your code that you intended to do nothing for certain events.

In light of that, you have a couple of options:

Option 1 - add the default

default:
  break;

This suppresses the warning, and makes it clear that you don't intend to handle the other event types here.

Option 2 - list each value

List each event type, followed by a break. This is also explicit, and has the added bonus that, should you ever add an event type, the compiler will once again warn you that your switch is incomplete. This can be valuable when you have many switch statements, some of which need to be modified to do something new when an enum value is added.

What about a series of if statements?

I would not recommend using a series of if statements here. A switch is clearer, reduces the amount of typing, and (as you've seen) can produce better compiler warnings for cases you omitted.

Andrew
  • 5,212
  • 1
  • 22
  • 40
  • Nice argumentation of Option 2, however in my simply case where I am interested only of particular events handling, I think Option 1 is my case. Thank you. – bluszcz May 16 '16 at 13:11
  • Option 1 did the job for me! – Colby Cox Apr 26 '17 at 03:14
  • For option 2 you could also list all event types and put `break` only after the last one. – Dino Nov 13 '19 at 08:59
  • 1
    @Dino that's certainly possible, and my original statement wasn't specific about _where_ the `break` should live - i.e. is each one followed by a `break`, or only the last one? I would, however, advise following each case with a `break`, as compilers tend to warn about fall-through in switch statements, and if one of them was ever edited to do something meaningful, the fall-through logic may accidentally not be updated, which could cause problems. A few more characters typed, to save ourselves from pain later! – Andrew Nov 13 '19 at 10:26
  • It seems to me with '-DSHOW_WARNINGS=1', GNU make version 3.81 doesn't suppress warnings with Option 1. These lines are still seen. ```11: warning: enumeration value 'ABC' not explicitly handled in switch [-Wswitch-enum]. 11: note: add missing switch cases ``` – ywu Apr 27 '21 at 23:41