Contrib.social

Make fraction reduce optional after arithmetic operations

Open on Drupal.org →
Created on 25 March 2023, almost 2 years ago
Updated 30 March 2023, over 1 year ago

Problem/Motivation

Currently fractions are automatically reduced at the end of all arithmetic methods (add(), subtract(), multiply(), divide()).

In some cases, downstream code may not want the fraction to be reduced.

Proposed resolution

Make the reduce operation optional with a new bool $reduce = TRUE parameter on add(), subtract(), multiply(), and divide() methods.

The parameter should default to TRUE to maintain the current default behavior, so that existing usages do not change.

Remaining tasks

  • Add bool $reduce = TRUE parameter to add(), subtract(), multiply(), divide() methods, and wrap existing $this->reduce() logic in a conditional that checks the $reduce parameter.
  • Add automated tests.

User interface changes

None.

API changes

Fraction class arithmetic operations (add(), subtract(), multiply(), divide()) will have an additional bool $reduce = TRUE parameter.

Data model changes

None.

✨ Feature request
Status

Postponed

Version

2.0

Component

Code

Created by

🇺🇸United States m.stenta

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @m.stenta
  • Comment almost 2 years ago →
    🇺🇸United States m.stenta

    For context, this came out of debugging this issue: 🐛 Quantity/unit price/total price auto-calculation rounding issue Fixed

    Making reduce optional is one potential solution to that problem. But it also just seems like a good option to provide generally.

  • @mstenta opened merge request.
  • Status changed to Needs review almost 2 years ago
  • Comment almost 2 years ago →
    🇺🇸United States m.stenta
  • Status changed to RTBC over 1 year ago
  • Comment over 1 year ago →
    🇺🇸United States m.stenta

    This is a simple change, and tests are passing. Setting this to RTBC.

    We should tag a new minor version when this is merged since it changes the Fraction class API. It's backwards compatible, so I don't think we need a new major version.

  • Status changed to Postponed over 1 year ago
  • Comment over 1 year ago →
    🇺🇸United States m.stenta

    On second thought, I'm going to postpone this for now. It changes the API of the Fraction class, so it really should be a major version change.

    And since there has been no actual demand or ask for this, I don't see any reason to prioritize a Fraction 3.x branch. When we do, we can include this.

    I was originally considering it as a solution to another issue ( 🐛 Quantity/unit price/total price auto-calculation rounding issue Fixed ), but that got resolved by other means.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024