Re: [API] Some more cleanup ideas

On Fri, 2012-11-30 at 18:53 +0100, Lubos Lunak wrote:
> On Friday 30 of November 2012, Stephan Bergmann wrote:
> > I'm not sure this is a good move.
...
> > Which leaves us with the benefit of shorter, less visually cluttered
> > declarations of C++ functions.  But, as I argue above, I am not sure
> > that is an overall benefit at all.

        I'm convinced it is a benefit :-) - and it's interesting; there have
been a string of decisions in the past where we have decided to go for
more readable code over (IMHO) gross verbosity:

-       rtl::OUString( RTL_USTRING_VERYLONGNAME( "foo" ) )
+       "foo"

        to me is a -massive- improvement in readability. More minor ones are:

-       rtl::OUString
+       OUString

        Many of these have been resisted with the same "no benefit" argument. I
believe that every redundant token we can clarify, and simplify makes
reading and getting into the code much easier for new people. Hackers
have to spend tons of their time reading and un-tangling code, so
maximum readability is key.

        In my view the inevitable line-wrapping / 80 char bustage that huge
long exception descriptions bring brings a significant impairment of
readability. What should read as a simple interface implementation (and
to be fair looks reasonably clean and readable as IDL) gets turned into
something much fouler in the header & impl. The css:: mangling helps in
some minor way with this - but I feel it is far better to hide as much
as humanly possible of that duplicate guff.

>  So, in practice, the specifications in our case are like writting out asserts 
> in the code, and the only questions that there should be are whether it makes 
> sense to have such asserts and whether they are worth the code clutter.

        Quite. Personally - I'm deeply unconvinced that we can afford to have a
code-base where all manner of unexpected side-effects remain un-checked
at compile-time etc. The "only call methods that throw exceptions
mentioned above" criteria is (to me) far too opaque to have any
reasonable chance of reviews and newbie implementers from following it -
worse manual checking of that sort of tedious detail, even if it is done
is highly error prone: and I suspect is done only one way. Who - when
changing a function checks all call-sites of it to see if they declare
that exception ? ;-)

        So - IMHO it would be -far- better to have a magic clang plugin to
build a database of what methods are called by what other methods, and
what signatures they have, and what exceptions they throw as the code
compiles - and post build to dump a list of offending methods: I suspect
we will have a lot of them to fix.

        Then leave that on on the tinderbox in perpetuity to enforce
cleanliness, allowing us to remove the signatures, make the code more
readable, and stop bothering about another set of constraints as we
write / review code :-) And of course, I'd love to write that plugin
myself if no-one beats me to it ;-)

        ATB,

                Michael.

-- 
michael.meeks@suse.com  <><, Pseudo Engineer, itinerant idiot