Date: prev next · Thread: first prev next last
2010 Archives by date, by thread · List index


On Tue, 30 Nov 2010 18:50:17 +0100, Lubos Lunak <l.lunak@suse.cz> wrote:
On Tuesday 30 of November 2010, Thorsten Behrens wrote:
Additionally, I think most classes don't necessarily need detailed
docs for all methods in the first place (which may also hurt later
merging from OOo), but would already benefit from a two-line
"mission statement" at class level (of course plus some module-level
overview of "what's inside").

 I beg to differ.

 This could be a significant factor for new possible contributors. While 
patches removing dead code or similar certainly help too, the codebase can 
move forward only by people writing new code, and that requires understanding 
of the existing code.

I am with Lubos here I have to say. I do see Thorsten's point on that
wrong documentation is worse than no documentation, and fixing wrong
APIs and function names would be preferred. I also see how it makes
merging worse. 

BUT, as a newcomers, I was very much overwhelmed with millions of lines
of code, many of which were not documented at all. And as a newcomer you
don't even know what is a core API and what is a seldomly used
thing. 

People who are not intimate with but rather intimidated
by the code base, do need these docs to steer around and understand the code.

Being able to look up in doxygen what a "bFull" parameter implies
and which effect it has in a function, would be a real boon. Let me give
a random real world example that I just pulled up by searching the code
base for a bFull parameter :):

In /libs-core/editeng/inc/editeng/unofored.hxx we define:
QuickFormatDoc( BOOL bFull=FALSE );

What does bFull mean? Not so quick? What portions will be formatted if
this is FALSE? Looking at the function it either calls
 pImpEditEngine->FormatFullDoc(); 
 or 
 pImpEditEngine->FormatDoc();

What the heck is the difference between those functions? Now I have to
go another layer deeper to those 2 (both undocumented!) functions.

All that FormatFullDoc does:

  for ( sal_uInt16 nPortion = 0; nPortion < GetParaPortions().Count(); nPortion++ )
    GetParaPortions()[nPortion]->MarkSelectionInvalid( 0, 
GetParaPortions()[nPortion]->GetNode()->Len() );
  FormatDoc()

Excuse me, what does the stuff before we end up in FormatDoc() actually
do? It must modify the document somehow as a sideeffect,because we call
FormatDoc without any parameter. And it seems to mark some selections as
invalid. So perhaps bFull=FALSE only works on selected text?

For that I need to dig into what GetParaPortions and ParaPortions
actually are and do, which is an *undocumented class* implemented here: 
libs-core/editeng/source/editeng/editdoc2.cxx.

I'd give up at this point, because after reading that much code, I had
forgotten what I wanted to do in the first place :). A simple docstring
in QuickFormatDoc, such as
/**
 * param bFull determines whether we need to reflow the whole document
 or only the pieces that are visible on the screen.
 */
would have saved me much time, and I could actually have improved some
code rather becoming a frustrated opengrok hunter..
(note this is a bullshit comment, as I *still* don't know what bFull
really does :)).

Sebastian

Attachment: pgpwnSe8ny6tc.pgp
Description: PGP signature


Context


Privacy Policy | Impressum (Legal Info) | Copyright information: Unless otherwise specified, all text and images on this website are licensed under the Creative Commons Attribution-Share Alike 3.0 License. This does not include the source code of LibreOffice, which is licensed under the Mozilla Public License (MPLv2). "LibreOffice" and "The Document Foundation" are registered trademarks of their corresponding registered owners or are in actual use as trademarks in one or more countries. Their respective logos and icons are also subject to international copyright laws. Use thereof is explained in our trademark policy.