On Sep 1, 2011, at 2:19 AM, Mohammad Elahi wrote:
> Changed function lcl_formatPersianWord to be more generic, and added support
> for some more numbering types:
> English word: one, two, three, ...
> English cardinal: first, second, third, ...
> English cardinal semi-word: 1st, 2nd, 3rd, ...
> Persian cardinal word.
>
> I used C++ macros, but do not know whether libreoffice community
> likes using it or not?
> Any feedback is welcomed.
First, I think extending this from Persian to English already shows the biggest
flaw of this approach: Do you want to extend in in this way for all languages
supported by LibO? I would consider such extension to additional languages a
localization task, a task that typically only consists of translating string
resources. Here, however, someone doing localization would need to add new
constants to NumberingType.idl and would need to add code to
defaultnumberingprovider.cxx. That does not feel right.
That said, concentrating on details of the code:
- At least I do not like macros very much. But at least DEFINE_WORD_TABLE is
local to a single .cxx.
- In C++, no need for
typedef struct {…} NumberWordTable;
Instead, use the shorter
struct NumberWordTable {…};
- "the second table is used for irregular cardinal numbers is not empty":
should probably read "if not empty"?
- In
sal_Unicode *(table1[2][8]);
the superfluous parentheses are IMO confusing, and the sal_Unicode data should
really be const, so rather
sal_Unicode const * table1[2][8];
- For the Persian characters (that cannot be given visibly in an ASCII-only
.cxx file, anyway) the practice of specifying sal_Unicode strings as sequences
of individual characters (a la {0x0635,0x062f,0}) appears acceptable. However,
for English strings, {'o','n','e',0} vs. "one" is hard to tolerate. Maybe all
the data should be specified as UTF-8 instead, using literal "…" strings (the
literal Persian UTF-16 characters like 0x0635 become a little harder to
maintain, having to encode them as UTF-8 like "\xXX\xYY"), and explicitly
converting to rtl::OUString when used.
-Stephan
_______________________________________________
LibreOffice mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
