Bug Hunting Session
Bug 113768 - Meaning of last VDB function argument ("Type") inverted in help tip and LO help
Summary: Meaning of last VDB function argument ("Type") inverted in help tip and LO help
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
Inherited From OOo
Hardware: All All
: medium normal
Assignee: Winfried Donkers (retired)
URL:
Whiteboard: target:6.0.0
Keywords:
Depends on:
Blocks:
 
Reported: 2017-11-11 09:02 UTC by Johnny_M
Modified: 2017-11-15 07:07 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments
VDB function test spreadsheet (see yellow cells) (28.19 KB, application/vnd.oasis.opendocument.spreadsheet)
2017-11-11 09:03 UTC, Johnny_M
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Johnny_M 2017-11-11 09:02:08 UTC
According to the requirement in [1] (ODF 1.2), the meaning of the last argument of Calc's VDB function is:
> noSwitch is an optional argument that you include if you do not want VDB to
> switch to straight-line depreciation for the remaining useful life.
> Normally, declining-balance switches to such a straight-line calculation
> when it is greater than the declining-balance calculation.

> If noSwitch is FALSE() or omitted, VDB automatically switches to straight-
> line depreciation when that is greater than declining-balance depreciation.
> If noSwitch is TRUE(), VDB never switches to straight-line depreciation.


Current situation:
------------------
That last argument currently has the following description in LO:

I) In the help tip on entering the VDB function in a cell in Calc (with cursor on the last function argument):
> Do not alter. Type = 1 denotes switch to linear depreciation, type = 0 do
> not switch.

II) In LO help ([2])
> Type = 1 means a switch to linear depreciation. In Type = 0 no switch is
> made.

III) In LO code:
- bType variable name in [4]
- nType variable name in [5]

The issue is that in the above cases I and II, the description of the argument is the opposite to the requirement. Although the logic seems to be implemented correctly in the code with respect to the values 0 (false) and 1 (true).


Proposal:
---------
For the cases I (help tip) and II (LO help) above:
- Correct the tip and the help content according to the requirement. (I.e., invert the meaning of the content.)
- Change the name of the argument from "Type" to "No_switch", as, e.g., in MS Office (see [2]). Which is close to name in the requirement.

For the case III above, as alignment:
- Rename the variables to bNoSwitch and nNoSwitch respectively


References:
-----------
[1] http://docs.oasis-open.org/office/v1.2/os/OpenDocument-v1.2-os-part2.html#__RefHeading__1018330_715980110
[2] https://support.office.com/en-us/article/VDB-function-dde4e207-f3fa-488d-91d2-66d55e861d73
[3] https://help.libreoffice.org/Calc/Financial_Functions_Part_Three#VDB

[4] https://gerrit.libreoffice.org/gitweb?p=core.git;a=blob;f=sc/source/core/tool/interpr2.cxx;h=d4c27a4d0fd5ccf73a1f81b46a0155b1135dd17e;hb=HEAD#l1798
[5] https://gerrit.libreoffice.org/gitweb?p=core.git;a=blob;f=sc/source/core/opencl/opinlinefun_finacial.cxx;h=38eb41db02db218f56b13630670b29c59c40567b;hb=HEAD#l1535


Correctness of the implemented logic tested on:
-----------------------------------------------
Version: 5.3.7.2
Build ID: 1:5.3.7~rc2-0ubuntu0.17.04.1~lo0
CPU Threads: 4; OS Version: Linux 4.10; UI Render: default; VCL: gtk3; Layout Engine: new; 
Locale: de-DE (en_US.UTF-8); Calc: group

Version 5.3.6.1 Portable on Win10 64-bit

And compared to MS Excel 2016
Comment 1 Johnny_M 2017-11-11 09:03:12 UTC
Created attachment 137678 [details]
VDB function test spreadsheet (see yellow cells)

The yellow highlighted cells in the attached test file can be used for manual testing of the help tip and the function logic.
Note: SLN is the straight-line depreciation being switched to.
Comment 2 Johnny_M 2017-11-11 09:16:45 UTC
@Eike, Winfried: What is your opinion on this?
Comment 3 Winfried Donkers (retired) 2017-11-11 16:10:21 UTC
I confirm that help(tip) text for the NoSwitch argument is the opposite of what it should be.
A fix will probably be in 2 parts:
-help tip text and internal renaming of variables in the code;
-help text.

@Johnny_M : given that you already dived into the source code, is it your intention to fix it yourself? If you want, I can point you to the help tip code location for VDB.
If you don't want to fix it, I can do it.
Comment 4 Johnny_M 2017-11-11 16:26:35 UTC
Thank you very much for the feedback, Winfried!

Since especially the help tip part (and LO help) would probably need compilation and testing of the result for visual glitches, etc., and I don't have a build environment, please feel free to take this. I'll be happy no matter who corrects it. :)
Comment 5 Commit Notification 2017-11-13 17:28:47 UTC
Winfried Donkers committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=76c2e3efe751d42f7b352bebdddfe33bd37d9e06

tdf#113768 Fix opposite argument description for Calc function VDB.

It will be available in 6.0.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 6 Commit Notification 2017-11-14 19:02:52 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/help/commit/?id=008c845819cd34a02fba16ee89c784d02d6740fa

Related: tdf#113768 VDB change Type parameter opposite to NoSwitch