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

Add isEqual() to the ObjectArithmetic interface. #396

Open
Beakerboy opened this issue Oct 20, 2020 · 8 comments
Open

Add isEqual() to the ObjectArithmetic interface. #396

Beakerboy opened this issue Oct 20, 2020 · 8 comments

Comments

@Beakerboy
Copy link
Contributor

Currently, all the classes in the Number namespace have methods name equals(), while Matrix has isEqual(). The polynomial class has nothing. Since PHP is loosely typed, I think we should approach adding this method to the interface with a unified philosophy that takes that into consideration. PHP has == as well as === for native comparisons. Should we have methods that compare value, and one that also compares type? Maybe IsEqual() and isIdentical()?

@markrogoyski
Copy link
Owner

Are you thinking you'd want like $rational->isEqual($arbitraryInteger) to possibly be true while $rational->isIdentical($arbitraryInteger) would be false?

@Beakerboy
Copy link
Contributor Author

Beakerboy commented Oct 20, 2020

I think that would require some sort of casting logic where everything would have to pass into a float.

I was thinking more like how a complex number could equal a float when the imaginary part is zero.

public function isIdentical($object): bool
{
    return get_class($this) == get_class($object) ? $this->isEqual($object) : false;
}

@markrogoyski
Copy link
Owner

Casting to float is probably doing to have erratic results.
For example:

php > echo 1/3;
0.33333333333333
php > var_dump(0.33333333333333 == 1/3);
bool(false)

The ObjectArithmetic interface is really for the object orchestrating the operations, with the presumption that they are all the same kind of object that implements that interface. It gets too complicated if you try to make this work across different types.

I think isEquals is all you need for now and it should probably fail if the types are not the same.

@Beakerboy
Copy link
Contributor Author

The ArbitraryInteger class already behaves this way though. If you say $object->equals(15) it will return true when $object = new ArbitraryInteger(15);

@markrogoyski
Copy link
Owner

It looks like it is comparing properties, after having maybe transformed a scalar into an object.

    public function equals($int): bool
    {
        $int = self::create($int);
        return $this->base256 == $int->toBinary() && $this->isPositive == $int->isPositive();
    }

@Beakerboy
Copy link
Contributor Author

The way it’s implemented does not matter. I’m asking what standard We want for allowed inputs and expected output. In the case of the ArbitraryInteger, it’s faster to convert an int into an ArbitraryInteger than dealing with potential overflows from doing the opposite. For a Complex number, I would probably say, if the input is numeric then check if $this has a zero for the imaginary part, then check if the real part matches the provided number. Polynomial would do something similar with it’s storage array. The RationalNumber would have to cast to a float.

@markrogoyski
Copy link
Owner

I think the Complex and ArbitraryInteger and maybe even the Polynomial if they have integers will work fine comparing to scalars, but the Rational is going to be the problem.

php > $rational = new MathPHP\Number\Rational(0, 1, 3);
php >
php > echo 1/3;
0.33333333333333
php > echo $rational->toFloat();
0.33333333333333
php > var_dump(0.33333333333333 == 0.33333333333333);
bool(true)
php > var_dump($rational->toFloat() == 0.33333333333333);
bool(false)

I think it is interesting to be able to compare the object and scalar representations of these numbers so I'd want to try it. So let's give make the isEquals smart and then you do have the use case to also add isIdentical. But there will be erratic behavior whenever floats are involved. Just keep that in mind.

@Beakerboy
Copy link
Contributor Author

Beakerboy commented Oct 21, 2020

When comparing floats, I would use the epsilon that is used throughout the library.

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

No branches or pull requests

2 participants