Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ROUND Accepts null, false, and true as First Parameter #1837

Merged
merged 7 commits into from
Feb 13, 2021
Merged

ROUND Accepts null, false, and true as First Parameter #1837

merged 7 commits into from
Feb 13, 2021

Commits on Feb 11, 2021

  1. ROUND Accepts null, false, and true as First Parameter

    Issue #1789 was addressed by PR #1799. In a follow-up discussion,
    it came to light that ROUND was not handling the unexpected case where the
    first parameter is an empty cell in the same manner that Excel does.
    Subsequent investigation showed that a boolean first parameter is permitted.
    I broadened my investigation to include the following related functions.
    - ROUNDUP
    - ROUNDDOWN
    - MROUND
    - TRUNC
    - INT
    - FLOOR
    - FLOOR.MATH
    - FLOOR.PRECISE
    - CEILING
    - CEILING.MATH
    - CEILING.PRECISE
    
    All of these allow a NULL first parameter, and all except MROUND allow boolean.
    For completeness, I will note that all treat null string as invalid.
    I suspect there are other functions which permit
    similarly unexpected parameters, but I consider them out of scope for this PR.
    
    CEILING.MATH and CEILING.PRECISE were unimplemented, and are now supported
    as part of this PR.
    
    The tests for each of these functions have been re-coded, though all the original
    test data is still included in the test cases, plus several new cases for each.
    The new tests now take place as a user would invoke the functions,
    through a spreadsheet cell rather than a
    direct call to the appropriate function within Calculation/MathTrig.
    Aside from being more realistic, the new tests are also more complete.
    For example, FLOOR.MATH can take from 1-3 arguments, and the existing tests
    confirmed that the function in Calculation could handle a single argument.
    However, the function list in Calculation.php erroneously set the number of
    arguments for FLOOR.MATH to exactly 3, so, if a user tried to get the calculated
    result of a cell containing FLOOR.MATH(1.2), the result would be an Exception.
    
    Aside from the parameter support, there are a few minor code changes.
    Ods, as well as Gnumeric, allows the omission of the second parameter for
    FLOAT and CEILING; Excel does not. A potential divide-by-zero error is
    avoided in CEILING, FLOOR, and FLOORMATH.
    
    I will note that it would probably be beneficial in terms of maintainability
    to break MathTrig up into many individual modules. The same would hold for the
    other Calculation modules. I would be willing to look into this if you agree
    that it would be worthwhile.
    oleibman committed Feb 11, 2021
    Configuration menu
    Copy the full SHA
    14d1531 View commit details
    Browse the repository at this point in the history
  2. Scrutinizer Changes

    One of the 2 new functions had Complexity C.
    oleibman committed Feb 11, 2021
    Configuration menu
    Copy the full SHA
    edc1ad9 View commit details
    Browse the repository at this point in the history
  3. Scrutinizer Again

    Erroneous return type in Docblock.
    oleibman committed Feb 11, 2021
    Configuration menu
    Copy the full SHA
    d3c64fc View commit details
    Browse the repository at this point in the history
  4. PHP-CS-Fixer Error

    Can't duplicate on my system. Trying an alternative.
    
    I think it's kind of tacky that php-cs-main step can fail,
    without indicating what member it failed in, let alone what line.
    oleibman committed Feb 11, 2021
    Configuration menu
    Copy the full SHA
    974239c View commit details
    Browse the repository at this point in the history

Commits on Feb 13, 2021

  1. Start Breaking Up MathTrig

    Move functions involved in this PR to their own classes.
    oleibman committed Feb 13, 2021
    Configuration menu
    Copy the full SHA
    f80ab85 View commit details
    Browse the repository at this point in the history
  2. Scrutinizer Recommendation

    Minor error - potential mis-typing.
    oleibman committed Feb 13, 2021
    Configuration menu
    Copy the full SHA
    13833cd View commit details
    Browse the repository at this point in the history
  3. Mark Baker Recommendations

    Retain deprecated versions of functions in MathTrig.
    Use explicit typing when possible.
    oleibman committed Feb 13, 2021
    Configuration menu
    Copy the full SHA
    151683e View commit details
    Browse the repository at this point in the history