[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-i18n-doc
Subject:    Re: KGeography needs your help
From:       Chusslove Illich <caslav.ilic () gmx ! net>
Date:       2005-08-11 2:57:46
Message-ID: 200508110457.49706.caslav.ilic () gmx ! net
[Download RAW message or body]


> [: Nicolas Goutte :]
> However if the class has its own private class d, like any good public
> KDE class, you do not care if d constains (in Qt4 classes) QString,
> Q3CString, QByteArray or std::string . And you do not have to change
> your binary compatibility.

While trying this approach, I even put a QString member variable outside of 
private class :) (The only other thing outside is a boolean, specifying if 
scripting is on or off.) I did this in order to avoid any unnecessary 
overheads when scripting is turned off, so that currently I even don't 
have to allocate the private class if scripting is off.

I also didn't feel a pressing need to eliminate QString. Everything else in 
KDE/Qt is rifled with it, and it was a previous choice for i18n. Even 
Stephan said in that other message that he doesn't mind "if i18n returned 
a class deriving from QString".

> Also books about OO programming tell to derive one class from another
> if there are of a same kind. That is not eaxctly the case here, as
> QString is a class for holding a string and KI18N is for processing
> translations.

Yes, I also feel some confusion with is-a/has-a distinction here, but one 
can say that KI18n is a QString, but a smarter one, which knows how to 
translate itself into another language :)

There is a practical problem too, programmer can do (and does) something 
like:

QString tmp = i18n("Blah, blah, %1");
...
tmp.arg(somearg);

Here the translation would be forced at the point of assignment, and script 
wouldn't be called at all because not all arguments have been supplied. 
But to avoid this, not only that KI18n shouldn't be inherited, but there 
shouldn't even be QString cast operator in non-inherited KI18n class. This 
would result in the cleanest solution, but breaking *all* current i18n 
code and programming habits:

QString msg = i18n("Blah, blah, %1").arg(somearg).t();

where method t() would stand for "it's assembled, translate it".

So, if we don't want to break all i18n code and programming habits, than 
perhaps solution with inheritance is not that different from the 
non-inherited with implicit cast to QString.

That problematic programming practice above would be treated as breaking 
the rule of "not splitting the messages", and requiring a fix, but 
luckilly a very easy one.

>> [: Chusslove Illich :]
>> There is one other thing that is bugging me though: QString doesn't
>> have virutal destructor [...] I have also declared operators new and
>> delete to be private [...]
>
> [: Nicolas Goutte :]
> See, because you have derived one class from another, you are starting
> to "peek and poke" around. That are clear signs that the derivation is
> bad.

Not only the allocation operators, but also arg() methods are overridden 
since they are not virtual in QString.

But these problems for themselves are not a cause of bad design alarm to 
me, because they exist for the reason that in C++ as a language this point 
is moot. Like, invariants of inheritance should be kept non-virtual, and 
other methods virtual, but QString actually has nothing virtual.

>> [: Chusslove Illich :]
>> 125 changes were needed because KGuiItem has a constructor that takes a
>> QString, and it was called by passing result of i18n to it.
>
> [: Nicolas Goutte :]
> But then we can add a constructor with KI18N as parameter. [...]

I tried, but at some places the method which was taking result of i18n() 
call could either be invoked with a QString or KGuiItem, again making 
compiler refuse to make a choice (use implicit KI18n->QString, or implicit 
KI18n->KGuiItem?)

> [...] Or add an operator that returns KGuiItem.

This cannot be, as KI18n is in the kdelibs/kdecore which depends only on 
kdelibs/dcop, KGuiItem is higher up.

>> [: Chusslove Illich :]
>> Most of the rest of explicit casts (75) were needed due to invoking 
methods
>> local8Bit() and length() directly on result of i18n call.
>
> [: Nicolas Goutte :]
> You can have there funtions as members of KI18N. That is not the
> problem. But compared of directly deriving from QString, you are
> controlling the interface.

This I thought of, but I don't like it. If we are going to try to mimic 
QString with KI18n by adding methods same as those in QString, then we 
loose the advantage of not inheriting from QString in the first place.

> [: Nicolas Goutte :]
> [...] You must see if the change are more in a way to make the
> code better or if it is in the direction of making more hacks.

Well, as I said above, the only totally clean solution is to have 
non-inherited KI18n with that t() method, but that breaks everything. And 
since no new fancy stuff is to be expected on programming side (i18n calls 
should be used just as they have been used till now), it is perhaps 
excessively clean. So currently I am more in favor of inheriting from 
QString. Also, control-wise this doesn't make things any worse than they 
are now, as QString itself is returned from current i18n()'s.

Of course, this issue of inherited vs. non-inherited KI18n class is 
definitely one that kde-core-devel should comment on.

-- 
Chusslove Illich (Часлав Илић)
Serbian KDE translation team

[Attachment #3 (application/pgp-signature)]

[prev in list] [next in list] [prev in thread] [next in thread]