Re: [Libreoffice] [PATCH] More numbering types: one, two, three, ...

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