Education ClassRoom/Previous Logs/OpenOffice.org Coding Guidelines

[16:59] <thorsten> --- classroom in ~1 minute ---

[16:59] <thorsten> there's a little intro presentation - http://wiki.services.openoffice.org/w/images/7/72/C%2B%2B- codingstandards.pdf

[17:00] * fredus (n=fredus@138.219.77-86.rev.gaoland.net) has joined #education.openoffice.org

[17:00] <metrokid> thank you for the file

[17:00] <thorsten> ok, hi everybody,

[17:01] <metrokid> hi

[17:01] <ericb2> thorsten: hello. Thanks for accepting to manage this ClassRoom.

[17:01] <thorsten> I'd like to start with the ooo coding standards classroom now,

[17:01] <Guillaume[ecn]> Hello

[17:01] <ericb2> @ all  :

[17:02] <ericb2> we use to start with ~20 to 30 minutes of presentation, then questions

[17:02] <ericb2> but things can be adapted, of course

[17:02] <ericb2> thanks for your attention, and let's start :-)

[17:02] <thorsten> thanks ericb2 for the invitation,

[17:03] <thorsten> let's start with a quick motivational presentation about why coding standards & what for ;)

[17:03] <thorsten> #slide 2:

[17:03] <thorsten> has the link to the c++ coding standards for OOo,

[17:03] <thorsten> which I'll mostly focus on

[17:04] <thorsten> there are coding guidelines for perl & java as well,

[17:04] <thorsten> but ~90% of OOo is C++, so... ;)

[17:04] <thorsten> everybody able to see the media?

[17:04] <Guillaume[ecn]> yep

[17:04] <thorsten> good :)

[17:04] <thorsten> slide #3

[17:04] <IZBot> no issue with number 3

[17:04] <IZBot> no issue with number 3

[17:05] <thorsten> haha

[17:05] <thorsten> so, you can read the content for yourself -

[17:05] <thorsten> there's a bunch of reasons to have coding standards,

[17:05] * metrokid reading Origins of Standards

[17:05] <thorsten> for me, one of the most important is to have a common vocabulary,

[17:06] <thorsten> and, especially for c++, to limit the variations, i.e.

[17:06] <thorsten> have a somewhat bounded problem space, when trying to express a certain kind of functionality

[17:06] <thorsten> like - use c-like, all-macros kind of coding vs. using boost -

[17:07] <thorsten> if that's mixed, people have an even harder time reading other's code

[17:07] <thorsten> a second thing is, and that's prolly best shown by this page:

[17:08] * mmu_man has quit ("Vision[0.9.7-Z-101305]: i've been blurred!")

[17:08] <thorsten> http://wiki.services.openoffice.org/wiki/Writer_Code_Conventions

[17:09] <thorsten> to give best-practice examples of how other people solved generic problems

[17:09] <thorsten> @ all: questions so far?

[17:09] <thorsten> cool.

[17:10] <thorsten> so, we based this on some standard literature, and experience we had in OOo,

[17:10] <thorsten> but, like, this did not fall from the skies :)

17:10] <thorsten> slide #4, btw

[17:10] <IZBot> no issue with number 4

[17:11] <thorsten> slide 5:

[17:11] <thorsten> you speak up when I'm too fast or too slow, ok?

[17:11] <Guillaume[ecn]> k

[17:11] <ssaboum> ok

[17:11] <thorsten> cool

[17:11] <thorsten> so,

[17:11] <metrokid> ok

[17:11] <thorsten> having rules is all nice & sweet, but

[17:12] <thorsten> if you don't actually use them, e.g. by reviewing code for them,

[17:12] <thorsten> there's mostly theoretical value in them :)

[17:12] <thorsten> thus, coding standards & code reviews go more or less hand in hand.

[17:12] <thorsten> first, you learn the rules by checking for them,

[17:13] <thorsten> second, when doing a review, you need something to check for, right?

[17:13] * aude86_2008 (n=aude86_2@138.219.77-86.rev.gaoland.net) has joined #education.openoffice.org

[17:13] <thorsten> slide 6:

[17:13] <thorsten> that's how a code review could work -

[17:14] <ssaboum> http://wiki.services.openoffice.org/w/images/7/72/C%2B%2B-codingstandards.pdf

[17:14] <thorsten> thx ssaboum

[17:14] <aude86_2008> thank you ssaboum

[17:14] <ssaboum> lol sorry (someone didn't hav it )

[17:14] <thorsten> we're on slide 6 - how a code review against the standards could work

[17:15] <thorsten> don't spend too much time on it, one hour has proven manageable,

[17:15] <thorsten> and take turns in reviewing.

[17:15] <thorsten> take the review as suggestion, not correction -

[17:16] <thorsten> if somebody spots something in your code, actually, you might be right -

[17:16] <thorsten> OTOH, if you spot something in other people's code, assume, that *he* might be right, not you,

[17:16] <thorsten> so:

[17:16] <thorsten> it helps to present reviews in a non-insulting way :)

[17:17] <metrokid> ^_^

[17:17] <thorsten> slide 7:

[17:17] <thorsten> we would love feedback on the rules,

[17:18] <thorsten> if you miss important stuff, or if you thing something is nowadays a no-brainer, let us know -

[17:18] <thorsten> or, change it right away, it's a wiki :)

[17:18] * frouvier (n=frouvier@43.215.194-77.rev.gaoland.net) has joined #education.openoffice.org

[17:18] <thorsten> any questions so far, about the motivation & goals?

[17:19] <thorsten> cool,

[17:19] <thorsten> then, I'd like to move over to a concrete example:

[17:19] <ssaboum> :-)

[17:19] <thorsten> could everybody please view this page:

[17:19] <thorsten> http://wiki.services.openoffice.org/wiki/Cpp_Coding_Standards/GEN

[17:20] <Guillaume[ecn]> done

[17:20] <thorsten> ok, these are 8 easily checkable rules,

[17:20] <thorsten> in fact, each one of those topic pages is suitable for a code review,

[17:21] <thorsten> i.e. don't review for everything (at first), go for one topic.

[17:21] <thorsten> please, everybody quickly skim over it,

[17:21] <thorsten> and in like 3 minutes, we'll try and review concrete code with that

[17:22] <thorsten> ok: code is here:

[17:22] <thorsten> http://svn.services.openoffice.org/ooo/cws/canvas06/svx/source/svdraw/svdobj.cxx

[17:22] <thorsten> and

[17:22] <thorsten> http://svn.services.openoffice.org/ooo/cws/canvas06/svx/inc/svx/svdobj.hxx

[17:23] <ssaboum> ouch ...

[17:23] <thorsten> we go for the class SdrObject ctor

[17:23] <thorsten> yeah, it's kinda nasty code :)

[17:24] <ericb2> comments in German :)

[17:24] <metrokid> Warum ?

[17:24] <thorsten> yep, the example is hand-picked for the effect

[17:24] <thorsten> metrokid: why what?

[17:24] <ssaboum> for the french people out there ctor is "contructeur"

[17:25] <metrokid> Kommentare in Deutsch

[17:25] <thorsten> metrokid asks why the comments are in German -

[17:25] <Guillaume[ecn]> thx for the translation ^^

[17:25] <thorsten> and the answer is, this is code from StarDivision time,

[17:25] <thorsten> when dev was exclusively German

[17:25] <ssaboum> nice...

[17:26] <ericb2> thorsten: is the order of the 8 rules http://wiki.services.openoffice.org/wiki/Cpp_Coding_Standards/GEN the suggested order, or is it a random order ?

[17:26] <thorsten> more or less random,

[17:26] <ericb2> thorsten: ok

[17:26] <thorsten> there is no real precendence

[17:26] <ericb2> thorsten: optimize later, maybe ? :)

[17:26] <thorsten> ok, everybody looking at SdrObject::SdrObject() now?

[17:27] <ssaboum> nopeµ....

[17:27] <fredus> nope

[17:27] <thorsten> let's check for "Initialize Everything Immediately (Init)" in that method,

[17:27] <thorsten> tell me when you're ready

[17:28] <ssaboum> can you give a line please ?

[17:28] * ChanServ gives channel operator status to sm|CPU

[17:28] <Guillaume[ecn]> 441

[17:28] <metrokid> yep, need it too

[17:28] <Guillaume[ecn]> i think...

[17:28] <thorsten> yep

[17:28] <ssaboum> thanks guillaume ...

[17:28] <fredus> thx

[17:28] * metrokid at 441

[17:29] <Guillaume[ecn]> np

[17:29] <thorsten> cool, so, anybody spotting any violations of the Init rule?

[17:29] <Guillaume[ecn]> mnLayerID(0),?

[17:29] <ssaboum> ok

[17:29] <ssaboum> done

[17:29] <Guillaume[ecn]> mpSvxShape(0)?

[17:30] <thorsten> well, _those_ are initialized,

[17:31] <thorsten> but what about e.g. aOutRect ?

[17:31] <thorsten> (that's in the header, line 444

[17:31] <ssaboum> humm

[17:32] <thorsten> well - that member is a class, and has a default ctor.

[17:32] <thorsten> but that's not very readable, and one has to know a lot of context,

[17:33] <thorsten> so, the recommendation is to explicitely initialize *all* class members in the class initializer list,

[17:33] <thorsten> which is this list starting in the cxx, line 441

[17:33] <thorsten> that *also* include all those bSomething vars, which are, in this case,

[17:34] <thorsten> initialized *later* in the ctor body, for no real reason.

[17:34] <thorsten> it all boils down to maintainability -

[17:34] <thorsten> if you have all members listed in the initializer list, you can simply count them,

[17:35] <thorsten> and ensure that everything is properly initialized,

[17:35] <thorsten> and ensure that everything is properly initialized,

[17:35] <thorsten> was that halfway understandable? ;)

[17:36] <ssaboum> yep i got it (almost :-) )

[17:36] <thorsten> cool,

[17:36] <metrokid> got, it

[17:36] <thorsten> so, checking for the "init" item, we'd suggested the code owner to slightly refactor this ctor,

[17:37] <thorsten> and move the bSomething initialization to the initializer list.

[17:37] <thorsten> next item to check:

[17:37] <thorsten> " Clear Origin of Local Data (LocalData)"

[17:38] <thorsten> metrokid: could you give an example for a naming scheme that distinguished local vs. non-local data?

[17:38] <metrokid> ....

[17:38] <ericb2> thorsten: Impl...  ?

[17:38] <ericb2> for locale

[17:38] <thorsten> yeah, that's common for local functions,

[17:39] <thorsten> for vars, the usual scheme is

[17:39] <thorsten> for class member variable, prefix an "m_" or only an "m",

[17:39] <thorsten> so, for a string, have

[17:39] <thorsten> m_aString as a class member,

[17:39] <thorsten> and aString as a local var.

[17:39] <ssaboum> ahhhhhh (the ah you sound when you get something )

[17:40] <thorsten> similarly, e.g. "g_" for globals,

[17:40] <thorsten> and "i_", "io_", "o_" for parameters,

[17:40] <thorsten> these are recommendations,

[17:41] <thorsten> unfortunately, except for the "m"/"m_" thingit, most of the code does not (yet) follow that ;)

[17:41] <thorsten> so:

[17:41] <thorsten> looking at the SdrObject ctor again, what can we spot?

[17:41] <ssaboum> mbLineIsOutsideGeometry

[17:42] <ssaboum> two m.....Something

[17:42] <thorsten> yep

[17:42] <ssaboum> but i don't get

[17:42] <thorsten> that one is alright - m for member var, b for boolean,

[17:42] * froumi (n=fred@trinity.ac-grenoble.fr) has joined #education.openoffice.org

[17:42] * valeuf (n=valeuf@kimsufi.anandra.org) has joined #education.openoffice.org

[17:42] * irc.freenode.net gives channel operator status to froumi

[17:42] <ssaboum> where is the sal_False thing ?

[17:42] * metrokid have to think about naming scheme....

[17:42] <Guillaume[ecn]> ok