wpilibsuite / allwpilib

Official Repository of WPILibJ and WPILibC
https://wpilib.org/
Other
1.08k stars 610 forks source link

Remove empty defaults from switch statements and expressions #6921

Open spacey-sooty opened 4 months ago

spacey-sooty commented 4 months ago

This is actually an anti pattern for switch statements as it prevents compile time checking of exhaustiveness. For switch expressions this is functionally equivalent to having no default.

rzblue commented 4 months ago

In java switch statements are not checked for exhaustiveness, and non-enum values generally can't be checked for exhaustiveness. It's generally considered good practice to have a default to make it clear what happens with all other values. Switch expressions obviously must be exhaustive, so for non-enum values you need a default case.

Is this for c++ specifically?

spacey-sooty commented 4 months ago

No this is also for Java, by adding an empty default to a switch expression you add absolutely nothing to the expression. If you were to instead add a default which does something (eg logs an invalid state) this would make sense but this is not what WPILib does right now.

rzblue commented 3 months ago

Perhaps instead of removing all default cases we should throw an exception or log a warning where appropriate, or add a comment indicating why it's ok that the default was reached. IMO not having a default is only fine where it's obvious why unhandled cases can be safely ignored.

by adding an empty default to a switch expression you add absolutely nothing to the expression.

Are you referring to a statement here instead? Expressions can't have empty cases