Hi Winfried, On Thursday, 2012-04-26 13:55:14 +0200, Winfried Donkers wrote:
I have added the formula 'datedif', as defined in ODF 1.2 to calc (see diff).
Hey, great!
But there are some items I would like to get advise on: 1. parameter fmt in datedif( date1; date2; fmt ) defines the output (d, m, y, yd, ym, md for days, months etc). In my code, the users needs to enter "d" (i.e. use the double quotes around). IMHO, that is not user-friendly. Can anyone tell me how to solve that?
Well, this is how the function is defined, the unit/format parameter is a text string.
2. the calculation in DateDif(..) uses mean values for days in year (365.2425) and for days in month (30.4369). That will in some instances lead to -seemingly?- incorrect results. Should I do something about that, i.e make the code date-concious?
Yes please, make it leap year aware and use correct values for days in months.
3. The various defined values for paramter fmt (d, m, y, yd, ym, md)
are 'hard' in the code ("d", "m", "y", "yd", "ym", "md"). This does
not look neat to me. is there a preferred way of doing such things?
No, hard coded is fine in this case as the values are not to be translated. However, string comparison should be case insensitive to allow also upper case values. IMHO Excel does that. Btw, do you have access to an Excel version to compare?
4. Currently, when an undefined value of parameter fmt is passed, the function returns 0 instead of an error message tp the user. Can anyone tell me how to implement a correct and user-friendly handling of incorrect parameter values?
Throw a lang::IllegalArgumentException, the interpreter handles that.
5. ODF 1.2 defines DATEDIF, but the languages I've seen using this function use DATEDIFF. Why is ODF different (or isn't it)?
The function originates from Lotus 1-2-3 where it was called DATEDIF, MS-Excel called it the same, and ODF just adopted that name. The DATEDIFF function you mention is something different (VBA?). Now some nitpicks on your code ;)
+++ b/sc/source/core/tool/addinhelpid.cxx
Is there a specific reason why you implemented this in the Analysis-AddIn? I don't recall exactly whether this function was provided in Excel as an AddIn or built-in. If as AddIn it makes sense to implement it in our AddIn for import/export from/to Excel, if not then a built-in function may be easier.
+++ b/sc/source/filter/oox/formulabase.cxx
+ { "DATEDIF", "DATEDIF", 485, NOID, 3, 3, V, { RR },
FUNCFLAG_EXTERNAL },
How did you determine the BIFF12 function identifier value 485?
+++ b/scaddins/source/analysis/analysis.cxx
+long SAL_CALL AnalysisAddIn::getDateDif( sal_Int32 nStartDate, sal_Int32 nEndDate, const STRING&
aFormat ) THROWDEF_RTE_IAE
+{
+ long lRet = GetDateDif( nStartDate, nEndDate, aFormat );
+ RETURN_FINITE( lRet );
+}
Please don't use machine specific type long in API calls, use sal_Int32 instead.
+++ b/scaddins/source/analysis/analysis.hxx + virtual long SAL_CALL getDateDif( sal_Int32 nStartDate, sal_Int32 nEndDate, const STRING& aFormat ) THROWDEF_RTE_IAE;
Here as well, use sal_Int32 instead of long.
+++ b/scaddins/source/analysis/analysis_deffuncnames.src
+ StringArray ANALYSIS_DEFFUNCNAME_DateDif
+ {
+ ItemList =
+ {
+ < "DATUMDIFFERENZ"; >;
+ < "DATEDIF"; >;
+ };
+ };
Is DATUMDIFFERENZ the name in a localized German Excel? I didn't find anything on that. These names should exactly match what Excel uses to be able to import localized AddIn names (which unfortunately they were in Excel). This is a reason why I asked if it was necessary to implement this function as an AddIn. Btw, when referring freedesktop.org bug numbers in commit summaries or source code please use our convention fdo#..., so fdo#44456 here. Thanks Eike -- LibreOffice Calc developer. Number formatter stricken i18n transpositionizer. GnuPG key 0x293C05FD : 997A 4C60 CE41 0149 0DB3 9E96 2F1A D073 293C 05FD
Attachment:
pgpEpthU7vas5.pgp
Description: PGP signature