Added section to style guide. http://wiki.blender.org/index.php/Dev:Doc/CodeStyle#Switch_Statement
Blenders style checker script can test this is being followed so we can avoid making these mistakes in the future. I'll check on adding this. On Mon, Jul 15, 2013 at 7:50 AM, Campbell Barton <[email protected]> wrote: > On Mon, Jul 15, 2013 at 1:17 AM, Dalai Felinto <[email protected]> wrote: >> Are we using the inline cases a lot in the current code? I'm not sure if >> it's a common practise elsewhere, but I find a bit strange: >> >> """case OB_CURVE: case OB_FONT: case OB_SURF:""" > > > Its not so common and mostly I would prefer devs don't do this, but in > the few cases we use, I think its OK - so I didn't want to exclude it > from the style-guide, eg: > > ./source/blender/windowmanager/intern/wm_event_system.c > > switch (event.type) { > case LEFTSHIFTKEY: case RIGHTSHIFTKEY: > ... > > ./source/blender/editors/interface/interface_handlers.c > switch (event->type) { > case ONEKEY: case PAD1: > ... > > Also see OB_DATA_SUPPORT_ID_CASE in source/blender/makesdna/DNA_object_types.h > > >> But +1 for using a style, whatever we pick. >> -- >> Dalai >> On Jul 14, 2013 9:23 AM, "Sergey Sharybin" <[email protected]> wrote: >> >>> Me also agreed. I've missed such a thing in our guidelines couple of times >>> already, actually. >>> >>> >>> On Sun, Jul 14, 2013 at 5:39 PM, Brecht Van Lommel < >>> [email protected]> wrote: >>> >>> > Agreed, the proposed code style is fine with me. >>> > >>> > On Sun, Jul 14, 2013 at 5:44 AM, Campbell Barton <[email protected]> >>> > wrote: >>> > > Yesterday I went over missing break's in switch statements and found a >>> > > surprising number of real bugs/errors in code (over 10), some rather >>> > > bad errors like boids 'random' behavior option, falling through to >>> > > 'average'. >>> > > >>> > > We didn't yet decide on details for switch() statements in our current >>> > > style-code [1], but I think doing so would help avoid more mistakes >>> > > like this as well as making code easier to follow. >>> > > >>> > > Most simple errors are not hard to spot errors but some code grows >>> > > over time - we had a mistake with a switch nested inside a switch for >>> > > eg. >>> > > >>> > > Note that this proposal is mostly what we do now anyway, so no large >>> > > code cleanup commits needed for this and proposal is minimal. >>> > > >>> > > >>> > > Proposal: >>> > > >>> > > - be consistent with placement of break, indent and include within >>> > braces. >>> > > >>> > > - intentional fall-through after statements in a case _must_ be >>> > commented. >>> > > (however successive case statements with no code can go without >>> > comments). >>> > > >>> > > >>> > > Heres an image:[2] and text:[3] of what I think is reasonable and an >>> > > example of whats not below. >>> > > >>> > > For an example of mixed up 'break' placement in current code, see >>> > > BKE_object_minmax in object.c >>> > > >>> > > >>> > > [1]: http://wiki.blender.org/index.php/Dev:Doc/CodeStyle >>> > > [2]: http://www.graphicall.org/ftp/ideasman42/bad_switch_example.png >>> > > [3]: http://www.graphicall.org/ftp/ideasman42/bad_switch_example.c >>> > > >>> > > -- >>> > > - Campbell >>> > > _______________________________________________ >>> > > Bf-committers mailing list >>> > > [email protected] >>> > > http://lists.blender.org/mailman/listinfo/bf-committers >>> > _______________________________________________ >>> > Bf-committers mailing list >>> > [email protected] >>> > http://lists.blender.org/mailman/listinfo/bf-committers >>> > >>> >>> >>> >>> -- >>> With best regards, Sergey Sharybin >>> _______________________________________________ >>> Bf-committers mailing list >>> [email protected] >>> http://lists.blender.org/mailman/listinfo/bf-committers >>> >> _______________________________________________ >> Bf-committers mailing list >> [email protected] >> http://lists.blender.org/mailman/listinfo/bf-committers > > > > -- > - Campbell -- - Campbell _______________________________________________ Bf-committers mailing list [email protected] http://lists.blender.org/mailman/listinfo/bf-committers
