Hi,
Thanks for the design update. I will review and get back to you with more
detailed feedback. Here are some quick thoughts:
On Fri, 10 Jun 2022 at 12:55, Sumanth Rajkumar <rajkumar.suma...@gmail.com>
wrote:
> Hi Alex and Giles,
>
> Thanks for the feedback.
>
> 1) Backward Compatibility and Complex Interface
> Yes. I understand the backward compatibility requirement and my goal is to
> be fully backward compatible.
>
> Fortunately, the existing Complex class has private constructors and so it
> is possible to refactor it as an interface.
> I was able to make the change along with a ComplexCartesianImpl class and
> run all unit tests successfully. I did not have to make any changes to unit
> tests.
> "mvn test" runs successfully on my local machine after the changes
>
'mvn test' will not run binary compatibility checks. You should try
building with the default goal: 'mvn'. This will run a binary
compatibility check japicmp:cmp in the 'verify' phase as it requires the
packaged jar file.
>
> We could split complex unary operators into two primitive functions (
> ToDoubleFunction<Complex>) one returning the real part of result and other
> for imaginary part
>
> interface ComplexFunction {
> ToDoubleFunction<Complex> getReal() ;
> ToDoubleFunction<Complex> getImaginary() ;
> }
>
>
This has concerns for efficiency. Look at some of the more involved
functions and you will see that there will be a lot of repeat computation
if you pass in the same complex number twice.
And for example the Conjugate implementation would look like this
> ComplexFunction conj = new ComplexFunction2() {
> @Override
> public ToDoubleFunction<Complex> getReal() {
> return complex -> complex.real();
> }
> @Override
> public ToDoubleFunction<Complex> getImaginary() {
> return complex -> -complex.imag();
> }
>
> };
> };
>
Quite verbose. However, can you even use a lambda function here?
>
> And the functions used like below in Complex and ComplexList
>
> Complex {
> // default implementation are immutable always returning new instance
> to maintain b/w compatibility
> default Complex applyFunction(ComplexFunction function) {
> return
>
> Complex.ofCartesian(function.getReal().applyAsDouble(this),function.getImaginary().applyAsDouble(this));
> }
> }
> }
>
> ComplexList {
> ..
> // applies changes in place
> void forEach(ComplexFunction fun) {
> ToDoubleFunction<Complex> realFunc = fun.getReal();
> ToDoubleFunction<Complex> imgFunc = fun.getImaginary();
> ComplexCursor cursor = new ComplexCursor();
> while (cursor.index < r.length) {
> cursor.apply(realFunc.applyAsDouble(cursor),
> imgFunc .applyAsDouble(cursor));
> cursor.index++;
> }
> }
> ...
> }
>
> Does this make sense or we just stick to the original interface that
> includes ComplexResult<R>?
>
I think a function for a complex number requires a real and imaginary input
and somewhere to put the real and imaginary answer. How to express this in
an interface that allows chaining is the tricky part.
Alex
