On Monday 08 of December 2014, Bjoern Michaelsen wrote:
Hi Lubos,
On Fri, Dec 05, 2014 at 06:46:17PM +0100, Lubos Lunak wrote:
On Thursday 04 of December 2014, Michael Meeks wrote:
* Large scale renames (Kendy)
...
+ if cleanup there; perhaps some improved naming too.
http://qt-project.org/wiki/API-Design-Principles#d8bc4b5cb3e68ae6e38b29e3
71b7f734 would be a very worthwhile reading here.
good link, thanks! I think the problem -- at least in Writer -- is a bit
deeper, no only naming: the classes in sw/ have somewhat muddy purposes and
arent too well defined in their scope. The naming is just the topping on
the cake (What is a SwFmtFrmSize and how is (if at all) it related to a
SwFrmFmt?).
I would agree that unfortunately the problem indeed is deeper, to the core
issue that code written poorly in some way attacts more code written poorly.
So when we inherited this codebase from OOo, we also inherited code that's
poorly named, poorly commented, poorly documented (et cetera). And with that,
we also inherited a culture where all that is perfectly fine and acceptable,
which was realistically inevitable if we wanted to actually get something
done with the code.
The bad part is that since it's fine and normal to have such poorly done
code, it's also fine to keep with that tradition and continue producing new
code that has the same flaws. Just look at some of the newly written code,
such as Clang plugins (I explicitly documented the purpose of each of mine
ones, but when I looked recently I either could guess from the name or
decipher it from the code) or the OpenGL VCL backend (the main class there
has functions DrawLine() and drawLine(), and yes, they differ).
So if you treat this only as a problem of some localized code, you may end up
in a situation of fixing up old code while new code gets written in a form
that immediately qualifies them for a such cleanup as well.
IMHO, the best way out of this mess would be to:
1/ find groups of around ~5 classes as a batch and define (and
doxygen-document) the single responsiblity of each of those well. It likely
makes sense to refer to the old "::SwFoo StarOffice/OpenOffice.org class
name" in doxygen too.
2/ move this set of classes a name matching the defined responsiblity in
namespace sw
That would mean we would try to start some consistent well-scoped naming in
namespace sw, while the global (top-level) namespace still contains the old
wild west naming. And them we would step by step grow the pocket by adding
stuff in a ordered fashion to it.
Given what I wrote above, I think this should start by first actually writing
down somewhere what the new proper state of things should be. Otherwise
nobody will know what the code ideally should look like, given that people
either can write code based on what the existing code looks like (where the
current code is pathetic when it comes to these criteria) or what some kind
of OOo resource like the OOo coding style says (which is just as pathetic) or
on their idea of what a good code should look like (which, to put it bluntly,
is probably not that good either, given the above). And then require it for
new commits. I'm generally not a big fan of being strict with rules (and it
certainly can be taken too far, try e.g. to submit a patch to Clang), but
then apparently the situation won't change on its own, if it hasn't in the
last 4 years.
Or, alternatively, we can just accept the fact that this codebase will in
some aspects suck forever.
--
Lubos Lunak
l.lunak@collabora.com
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.