Hi Daniel, On Tuesday, 2008-10-21 09:41:05 +0200, Daniel Rentz wrote:
> +BOOL lclConvertMoney( const String aSearchUnit, double& rfRate, int&
> rnDec )
Btw, aSearchUnit should be a const reference instead of a copy, so
const String & aSearchUnit
^^^
> +{
> +#define COUNT 16
> +#define CURRENCY( name ) \
> + (String::CreateFromAscii( RTL_CONSTASCII_STRINGPARAM( name ) ))
> + ConvertInfo aConvertTable[ COUNT ] = {
> + { CURRENCY( "EUR" ), 1.0, 2 },
>
>
> I am not sure (->Eike?) but initializing the strings this way may fail
> with certain compilers.
Yes, initializing static instances of class objects isn't guaranteed to
always work in the order assumed (well, it probably would in this case),
plus it adds unnecesary overhead when initializing the library's data.
One more thing: the often observed pattern
String aFoo( String::CreateFromAscii( RTL_CONSTASCII_STRINGPARAM( "literal" )))
is better written as
String aFoo( RTL_CONSTASCII_USTRINGPARAM( "literal" ))
with the advantage that an approriate ctor is called that already
includes the count of characters and no temporary instance of class
String is needed just to pass to the final String copy-ctor.
Note the U in USTRINGPARAM, without that you would get a different
parameter count and meaning!
However, the latest revision of the patch already changed the table
layout to use ASCII literals. The String::CreateFromAscii(...) still
applies though.
> Please do not #define the COUNT but calculate the array size after the
> array. In the future, more countries may convert to the Euro currency.
> This may be the source of errors when the next developer forgets to
> update the COUNT after adding a line to the list.
Which (w|sh)ould make the compiler complain about extra elements ...
anyway
> Better:
>
> const ConvertInfo aConvertTable[] = {
> ...
> };
>
> const int COUNT = sizeof( aConvertTable ) / sizeof( ConvertInfo );
Some nitpicks:
- Counts shouldn't be of type int but size_t instead.
- COUNT is a very common name that may clash sooner or later with some
includes, I suggest nConversionCount or such.
- Instead of sizeof(ConvertInfo) use sizeof(aConvertTable[0]), this way
one would not had to adapt that place in case the name of ConvertInfo
would have to be changed for whatever reason.
> if( maXclUrl.EqualsIgnoreCaseAscii( "\008EUROTOOL.XLA" ) )
>
> Note the characters \008 which encode the ASCII character 8.
Erm.. \010, just for the records, but we already clarified that on IRC ;-)
As a side note maybe interesting to others reading this: a literal
"\x08EUROTOOL.XLA" failed because the compiler takes \x08E as one entity
and thus produces the string 0x8E,"UROTOOL.XLA" ...
Eike
--
OOo/SO Calc core developer. Number formatter stricken i18n transpositionizer.
SunSign 0x87F8D412 : 2F58 5236 DB02 F335 8304 7D6C 65C9 F9B5 87F8 D412
OpenOffice.org Engineering at Sun: http://blogs.sun.com/GullFOSS
Please don't send personal mail to the [EMAIL PROTECTED] account, which I use
for
mailing lists only and don't read from outside Sun. Use [EMAIL PROTECTED]
Thanks.
pgplSq7Kq8DN5.pgp
Description: PGP signature
