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


On 01/13/2012 04:14 PM, Michael Meeks wrote:
On Fri, 2012-01-13 at 14:15 +0100, Stephan Bergmann wrote:
UNO *is* the tool to make functionality available to different
languages, to extensions, and to scripting facilities.

        Sure - of course. This API, on the other hand, is a C++ syntactic
sugar, so surely we can do things better in our native language
perhaps ? at least I think taking an efficiency hit to allow the
possibility of a later non-C++ configmgr implementation is unlikely to
pay dividends.

For me, its more of an "avoid an additional interface" than "fail to improve efficiency of C++ code" thing. Also, its not entirely a C++ coating only, parts of it can benefit other UNO bindings too (and at least in principle the type safe wrappings around the configuration items could also be moved to plain UNOIDL).

        I'd not seen any of this, so I'm just looking for the first time; I
have a number of queries (given that this will/should be used
everywhere).

* reading:
        workdir/unxlngi6.pro/CustomTarget/officecfg/registry/Office/Common.hxx

        I love the:
        namespace officecfg { namespace Office { namespace Common {

        ie. we skipped the org and the OpenOffice - which is cool; good
        to get over the over-namespacing legacy. My question would be -
        do we even need the 'Office' ? ;-) but ... it's prolly fine for
        now.

It strictly skips only the never-varying prefix "org/openoffice" (there are components like "Inet" and "Setup" next to "Office").

* string construction:

        struct UseOldExport: public unotools::ConfigurationProperty<UseOldExport, bool>  {
            static rtl::OUString path() { return 
rtl::OUString(RTL_CONSTASCII_USTRINGPARAM("/org.openoffice.Office.Common/InternalMSExport/UseOldExport"));
 }

        Since we're going to get a lot of these inserted, it might be
        rather a good plan to split a path() method from a key()
        method so the compiler can share the:
                /org.openoffice.Office.Common/.../
        for all the keys; that makes the call site slightly larger, but
        the resulting binary potentially rather smaller :-)

        it'd of course also be rather nice to keep the strings
        as const char *'s until we get them into some non-inlined code
        hiding that construction stuff inside some
        static ConfigurationWrapper::getPropertyValue(...) type method ?
        That way we get a single string (buffer) construction to build
        the path instead of one per call-site.

There's room for improvement, for sure. I'd save that for when larger chunks of client code have been adapted to actually use it. (The nice thing is that optimizations like the above can be introduced completely transparently and without the need to worry about any compatibility.)

* XComponentContext-age ...
            static ConfigurationWrapper const&  get(
                com::sun::star::uno::Reference<  com::sun::star::uno::XComponentContext>
        const&  context);

        Do we really need to pass this parameter to these methods ?

        We have a single configmr instance, it seems unlikely that we
        really need anything to help us find it surely ? either
        configmgr is there, or the world goes bang :-)

        If we're desparate to have it (it'd be nice to know what for),
        then we could we have a default-to-NULL pointer to a reference
        there ?

The UNO concept is to thread the (one and only) component context instance through the code, and in this world view comphelper::getProcessComponentContext is considered a hack to be cleaned up. Which one is more practical is surely debatable. (For the case at hand, calling comphelper::getProcessComponentContext internally in get() rather than passing in XComponentContext would evidently be less typing at the typical client side -- but then again, think about unit tests and the virtues of reducing global variables.) I did hesitate before making the parameter explicit, but then decided "in dubio for clean concepts."

        Anyhow - we should clearly create an EasyHack to drive this goodness
all through the code-base, I suspect it's highly susceptible to
wide-spread volunteer effort. Are you happy enough with it yet to do
that ?

As I already wrote: "I haven't announced this new C++ API yet, as some issues about change-notification are not yet completely thought out for it. Shame on me, should really do that soon."

Stephan

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.