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

Issue when extending an existing type with heritance #3336

Open
chrisbansart opened this issue Dec 17, 2024 · 1 comment
Open

Issue when extending an existing type with heritance #3336

chrisbansart opened this issue Dec 17, 2024 · 1 comment
Labels

Comments

@chrisbansart
Copy link

chrisbansart commented Dec 17, 2024

Hello,
I try to extend the Fraction type with a specific class to add the ability to use non reduced fractions. I would like to extend Fraction class to keep benefits of all existing types conversion and operations with Fraction when I don't overide them in my own custom Element_Fraction class.

Here the simple following class :

import { Fraction } from "mathjs";

export class Element_Fraction extends Fraction{
  isNonReduced = true;
  _originalS;
  _originalN;
  _originalD;

  constructor(n, d) {
    console.log("Element_Fraction constructor");
    super(n, d);
    this._originalS = math.sign(n) * math.sign(d);
    this._originalN = math.abs(n);
    this._originalD = math.abs(d);
    this.isNonReduced = true;
  }


  static add(a, b) {
    console.log("Element_Fraction add");
    const numerator =
      (a._originalS ?? a.s) * (a._originalN ?? a.n) * (b._originalD ?? b.d) +
      (b._originalS ?? b.s) * (b._originalN ?? b.n) * (a._originalD ?? a.d);
    const denominator = (a._originalD ?? a.d) * (b._originalD ?? b.d);
    return new Element_Fraction(numerator, denominator);
  }


  static subtract(a, b) {
    console.log("Element_Fraction subtract");
    const numerator =
      (a._originalS ?? a.s) * (a._originalN ?? a.n) * (b._originalD ?? b.d) -
      (b._originalS ?? b.s) * (b._originalN ?? b.n) * (a._originalD ?? a.d);
    const denominator = (a._originalD ?? a.d) * (b._originalD ?? b.d);
    return new Element_Fraction(numerator, denominator);
  }
//mandatory to add instance methods in addition to static methods to work in multiple context.
add(other) {
    return Element_Fraction.add(this, other);
  }

  subtract(other) {
    return Element_Fraction.subtract(this, other);
  }

...
}

I've also added the type declaration for my custom Element_Fraction

math.typed.addType({
  name: "Element_Fraction",
  test: function (x) {
    return x instanceof Element_Fraction;
  },
});
const add = math.typed("add", {
  "Element_Fraction, Element_Fraction": function (a, b) {
    return Element_Fraction.add(a, b);
  },
...
const subtract = math.typed("subtract", {
  "Element_Fraction, Element_Fraction": function (a, b) {
    return Element_Fraction.subtract(a, b);
  },
...

The bug

math.add((new Element_Fraction(2,4)),(new Element_Fraction(8,4)))

returns and Element_Fraction thus the implementation works well with the "add"

math.subtract((new Element_Fraction(2,4)),(new Element_Fraction(8,4)))

returns Fraction object, thus the implementation doesn't work.

I try with multiply, divide it occurs the same result. At the end, only "add" works, and it's pretty wierd.

Workaround
When I remove the heritance of Element_Fraction

export class Element_Fraction {

all operations works and returns my custom class. But unfortunaltly, I loose benefits of all exsting conversions and operations already implemented in Fraction.

To Reproduce

  1. Implement the class Element_Fraction and Element_Fraction type declarations
  2. Try : math.add((new Element_Fraction(2,4)),(new Element_Fraction(8,4))) // it returns Element_Fraction object
  3. Try : math.subtract((new Element_Fraction(2,4)),(new Element_Fraction(8,4))) // it returns Fraction object instead of Element_Fraction object

I may not have implemented my idea in the right way, so don't hesitate to tell me if it could be another approach to reach my initial objective.

Thanks,
Chris

@josdejong
Copy link
Owner

I haven't tested, but I think the issue may originate from Element_Fraction being both identified as Element_Fraction and as Fraction. The detection of Fraction is as follows:

mathjs/src/utils/is.js

Lines 55 to 57 in d394302

export function isFraction (x) {
return (x && typeof x === 'object' && Object.getPrototypeOf(x).isFraction === true) || false
}

The issue probably resolves when you change the Element_Fraction class such that isFraction returns false, I think setting a property .isFraction = false on the class should do the trick.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants