Rename FieldElement#add(T a) to FieldElement#plus(T a)

axelclk · November 21, 2023, 12:51pm

I think I’ve once discussed this already with @luc

But is there a chance to rename FieldElement#add(T a) to FieldElement#plus(T a) or FieldElement#sum(T a) in the future?

Hipparchus-Math/hipparchus/blob/f1948e10f100a5380c869c76b4f0e8f5f1004b68/hipparchus-core/src/main/java/org/hipparchus/FieldElement.java#L45


      
          /** Get the real value of the number.
           * @return real value
           */
          double getReal();
          
          /** Compute this + a.
           * @param a element to add
           * @return a new element representing this + a
           * @throws NullArgumentException if {@code a} is {@code null}.
           */
          T add(T a) throws NullArgumentException;
          
          /** Compute this - a.
           * @param a element to subtract
           * @return a new element representing this - a
           * @throws NullArgumentException if {@code a} is {@code null}.
           */
          T subtract(T a) throws NullArgumentException;
          
          /**
           * Returns the additive inverse of {@code this} element.

The signature add(T a) clashes with the java.util.List#add() interface method, so if I want to extend the interfaces into one class the List interface couldn’t be implemented correctly.
https://docs.oracle.com/javase/8/docs/api/java/util/List.html#add-E-

Does it make sense to open an issue on Github for this or is there no chance to do this refactoring in the future?

luc · November 21, 2023, 4:46pm

This was similar to thread Rename FieldElement add method.
My answer remains the same: it is technically possible and it will break a lot of existing code.

So I would again ask to other users and developers what they think about it, and I would comply to what the majority decides.

Serrof · November 21, 2023, 5:12pm

Hi,

I’m not in favour of such a change.

First, it’s not just FieldElement in Hipparchus that use this nomenclature, also Vector for example.

Second, it’s also the naming used by Java stuff like BigInteger.

Finally, as Luc said, it would break a lot of stuff, here and there (Orekit).

Best,
Romain.

axelclk · November 21, 2023, 8:01pm

Ok, so the main argument is that the change breaks to much stuff and therefore it doesn’t make sense to open an issue on github for a change in a future release?

The idea behind the question was, that if the #add() method will be moved to CalculusFieldElement interface and only renamed in FieldElement, the impact would only occur in classes BigFraction, BigReal and Fraction.
Is it then possible for other projects to manage such a renaming?

Or does it make more sense to introduce a more complete algebraic structure with a new “RingElement” interface like this one:

github.com

kredel/java-algebra-system/blob/master/src/edu/jas/structure/RingElem.java

/*
 * $Id$
 */

package edu.jas.structure;


/**
 * Ring element interface. Combines additive and multiplicative methods. Adds
 * also gcd because of polynomials.
 * @param <C> ring element type
 * @author Heinz Kredel
 */

public interface RingElem<C extends RingElem<C>> extends AbelianGroupElem<C>, MonoidElem<C> {


    /**
     * Greatest common divisor.
     * @param b other element.

This file has been truncated. show original

which inherits from interfaces AbelianGroupElem (with a #sum() method) and MonoidElem (with a #multiply() method)

luc · November 21, 2023, 10:14pm

If this is sufficient, it would be less disruptive for users, so yes I guess we could try that.
I also thought about all algebraic stuff a long time ago, but never implemented it as up to now we really had the need only for the field structure.

Serrof · November 21, 2023, 10:28pm

I think my point was missed. I didn’t say that the main argument against was API breaks. I meant that at the moment the naming across the whole Hipparchus library was consistent with itself as well as other Java components. And that’s important in my humble opinion.

Now, if what you’re proposing is to have an addition operator called sum for FieldElement and add for CalculusFieldElement, I find it very counterintuitive and NOT user-friendly, hence I strongly oppose it.

Not to mention that if we rename add into plus, we also need to update substract into minus.

Romain.

axelclk · November 26, 2023, 5:18pm

I would suggest such a change:

Should I create a PR and associated issue?

See also: algorithm - Simplifying expression trees - Stack Overflow

Serrof · November 26, 2023, 5:27pm

Oh so we also want to rename multiply now?

Honestly, if there must be an API change, I’d be more inclined to have methods like radd and (l)add to let the possibility to define different operations from the right and the left sides respectively, like in Python for instance.

Regards,
Romain

axelclk · November 27, 2023, 8:18pm

Ok I made some more experiments and the benefit is minimal.
I will not further try this refactoring.