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

List:       kde-i18n-doc
Subject:    Re: Proposal to improve support for ISO 3166 Country Codes in KLocale
From:       Andriy Rysin <arysin () gmail ! com>
Date:       2010-02-21 16:20:26
Message-ID: 3e02eb611002210820g580539qbd2a81961e033e9b () mail ! gmail ! com
[Download RAW message or body]

2010/2/21 Albert Astals Cid <aacid@kde.org>

> A Diumenge, 21 de febrer de 2010, Andriy Rysin va escriure:
> > 2010/2/21 Albert Astals Cid <aacid@kde.org>
> > > I don't see any i18n call there, how is translation going to work?
> > >
> > > Also getEntryList should be const and if you want to move it somewhere
> up
> > > in
> > > the stack you should not be using getFoo() in getter names, just foo().
> > >
> > > Also to be sincere i don't see the use of
> > >
> > >        static const char* iso_639;
> > >
> > >        static const char* attr_name;
> > >        static const char* attr_iso_639_2B_code;
> > >        static const char* attr_iso_639_2T_code;
> > >        static const char* attr_iso_639_1_code;
> > >
> > > apologies - I forgot to attach the client code example, here it is:
> >     // let's say I have the 3-letter language codes from the evdev.xml
> and
> > I need to put those languages into the combobox
> >     QList<QString> languages = ...
> >
> >     IsoCodes isoCodes(IsoCodes::iso_639);
> >     foreach(QString lang, languages) {
> >         const IsoCodeEntry* isoCodeEntry =
> > isoCodes.getEntry(IsoCodes::attr_iso_639_2B_code, lang);
> >         if( isoCodeEntry == NULL ) {
> >             isoCodeEntry =
> > isoCodes.getEntry(IsoCodes::attr_iso_639_2T_code, lang);
> >         }
> >         QString name = isoCodeEntry != NULL ?
> > i18n(isoCodeEntry->value(IsoCodes::attr_name).toUtf8()) : lang;
> >         layoutDialogUi->languageComboBox->addItem(name, lang);
> >     }
>
> I don't see why you need to expose iso_639, attr_name, etc to the user if
> they
> are fixed, you should just have different calls for them
>
> isoCodes.getEntry(IsoCodes::attr_iso_639_2B_code, lang);
> isoCodes.getEntry(IsoCodes::attr_iso_639_2T_code, lang);
>
> is much more ugly and error prone than
>
> isoCodes.get6392BCode(lang);
> isoCodes.get6392TCode(lang);
>
> since with your code i could write
>
> isoCodes.getEntry("iamabitdumbandwritingasillystringhere", lang);
>

I was trying to make the base class reusable, so yes with this class you can
hose yourself but it allows to reuse the class for any iso codes - not just
639. Plus this approach allows to work with those attributes dynamically
(i.e. if you want to provide a universal iso codes browser).

To provide nicer interfaces we would want to create subclasses (i.e.
Iso639Codes) which would have those attr_iso_* constants or even
getIso*Attr() methods - which would be a bit more friendly if client code
only cares about specific iso standard.

Just want to remind you that it was a quick solution for using languages in
kxkb. If this API to be exposed it would require a little bit of work. On
the other hand if we gonna wrap it in KLocale to provide API closer to what
John suggested it would not matter much as this class will be internal.

> I was thinking to embed i18n into the IsoCodes class itself first but then
> > realized it does not make much sense, so it's the client code which does
> > i18n and IsoCodes just manages the catalog. Let me know if this approach
> is
> > not good.
>
> To me it seems quite far on what John was thinking, i'd let him comment
> more
> on that though.
>

Again if we decide to wrap this code into KLocale it would not matter, as it
would just follow current KLocale approach for translations.

Andriy

[Attachment #3 (text/html)]

<div class="gmail_quote">2010/2/21 Albert Astals Cid <span dir="ltr">&lt;<a \
href="mailto:aacid@kde.org">aacid@kde.org</a>&gt;</span><br><blockquote class="gmail_quote" \
style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"> A \
Diumenge, 21 de febrer de 2010, Andriy Rysin va escriure:<br> <div><div></div><div class="h5">&gt; \
2010/2/21 Albert Astals Cid &lt;<a href="mailto:aacid@kde.org">aacid@kde.org</a>&gt;<br> &gt; &gt; I \
don&#39;t see any i18n call there, how is translation going to work?<br> &gt; &gt;<br>
&gt; &gt; Also getEntryList should be const and if you want to move it somewhere up<br>
&gt; &gt; in<br>
&gt; &gt; the stack you should not be using getFoo() in getter names, just foo().<br>
&gt; &gt;<br>
&gt; &gt; Also to be sincere i don&#39;t see the use of<br>
&gt; &gt;<br>
&gt; &gt;        static const char* iso_639;<br>
&gt; &gt;<br>
&gt; &gt;        static const char* attr_name;<br>
&gt; &gt;        static const char* attr_iso_639_2B_code;<br>
&gt; &gt;        static const char* attr_iso_639_2T_code;<br>
&gt; &gt;        static const char* attr_iso_639_1_code;<br>
&gt; &gt;<br>
&gt; &gt; apologies - I forgot to attach the client code example, here it is:<br>
&gt;     // let&#39;s say I have the 3-letter language codes from the evdev.xml and<br>
&gt; I need to put those languages into the combobox<br>
&gt;     QList&lt;QString&gt; languages = ...<br>
&gt;<br>
&gt;     IsoCodes isoCodes(IsoCodes::iso_639);<br>
&gt;     foreach(QString lang, languages) {<br>
&gt;         const IsoCodeEntry* isoCodeEntry =<br>
&gt; isoCodes.getEntry(IsoCodes::attr_iso_639_2B_code, lang);<br>
&gt;         if( isoCodeEntry == NULL ) {<br>
&gt;             isoCodeEntry =<br>
&gt; isoCodes.getEntry(IsoCodes::attr_iso_639_2T_code, lang);<br>
&gt;         }<br>
&gt;         QString name = isoCodeEntry != NULL ?<br>
&gt; i18n(isoCodeEntry-&gt;value(IsoCodes::attr_name).toUtf8()) : lang;<br>
&gt;         layoutDialogUi-&gt;languageComboBox-&gt;addItem(name, lang);<br>
&gt;     }<br>
<br>
</div></div>I don&#39;t see why you need to expose iso_639, attr_name, etc to the user if they<br>
are fixed, you should just have different calls for them<br>
<div class="im"><br>
isoCodes.getEntry(IsoCodes::attr_iso_639_2B_code, lang);<br>
</div><div class="im">isoCodes.getEntry(IsoCodes::attr_iso_639_2T_code, lang);<br>
<br>
</div>is much more ugly and error prone than<br>
<br>
isoCodes.get6392BCode(lang);<br>
isoCodes.get6392TCode(lang);<br>
<br>
since with your code i could write<br>
<br>
isoCodes.getEntry(&quot;iamabitdumbandwritingasillystringhere&quot;, \
lang);<br></blockquote><div><div><br>I was trying to make the base class reusable, so yes with this class \
you can hose yourself but it allows to reuse the class for any iso codes - not just 639. Plus this \
approach allows to work with those attributes dynamically (i.e. if you want to provide a universal iso \
codes browser).<br>

</div><br>
To provide nicer interfaces we would want to create subclasses (i.e. Iso639Codes) which would have those \
attr_iso_* constants or even getIso*Attr() methods - which would be a bit more friendly if client code \
only cares about specific iso standard.<br> <br>Just want to remind you that it was a quick solution for \
using languages in kxkb. If this API to be exposed it would require a little bit of work. On the other \
hand if we gonna wrap it in KLocale to provide API closer to what John suggested it would not matter much \
as this class will be internal.<br> <br></div><blockquote class="gmail_quote" style="border-left: 1px \
solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"> <div class="im">&gt; I was \
thinking to embed i18n into the IsoCodes class itself first but then<br> &gt; realized it does not make \
much sense, so it&#39;s the client code which does<br> &gt; i18n and IsoCodes just manages the catalog. \
Let me know if this approach is<br> &gt; not good.<br>
<br>
</div>To me it seems quite far on what John was thinking, i&#39;d let him comment more<br>
on that though.<br></blockquote><div> </div><div></div>Again if we decide to wrap this code into KLocale \
it would not matter, as it would just follow current KLocale approach for \
translations.<br><br>Andriy<br><div> </div> </div><br>



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

Configure | About | News | Add a list | Sponsored by KoreLogic