2
\$\begingroup\$

I've got 2 Java programs, with the aim of solving some matrix math that involves complex numbers (an EE problem). Both programs have main functions, the one with all the complex math operations has a main function to test the functions in that program itself. I rectified the issue of having an array of objects that have NULL instead of a complex value by writing a function to specifically do so. My questions are:

  1. Is there a better way to set object's values to zero(0+0i) instead of having to call my non_nullify function before almost every other function in the library

  2. Are there any other optimizations that can be done to make the program better in terms of modularity, performance, or other OOP terms

package com.PS1;
public class FormBus_builtin {
    public static void main(String args[]) {
        double[][] table = {{1,2,0,0.2},
                            {2,3,0,0.4},
                            {3,4,0,0.5},
                            {4,1,0,0.25}};
        int rows = table.length;
        int ne = rows;
        int cols = table[0].length;
        double[] sB_Array = column_extract(table,1);
        double[] eB_Array = column_extract(table,2);
        double[] resistanceArray = column_extract(table, 3);
        double[] reactanceArray = column_extract(table, 4);
        int sB,
            eB;
        double r,
                x;
        complex y_selfAdmittance,
                impedance;
        printArray(sB_Array,"starting bus array ");
        printArray(eB_Array, "ending bus array");
        printArray(resistanceArray, "resistance array");
        printArray(reactanceArray,"reactance array");
        complex[][] Y_bus = new complex[rows][cols];// creation of zero matrix for further substitution

        for(int k=0;k<ne;k++) {
            sB =(int) (sB_Array[k])-1;
            eB =(int) (eB_Array[k])-1;
            r = resistanceArray[k];
            x = reactanceArray[k];
            impedance = new complex(r,x);
            y_selfAdmittance = complex.reciproc(impedance);
            complex.println(y_selfAdmittance);
            Y_bus[sB][eB] = complex.subtract(Y_bus[sB][eB],y_selfAdmittance);
            Y_bus[eB][sB] = complex.subtract(Y_bus[eB][sB],y_selfAdmittance);
            Y_bus[sB][sB] = complex.add(Y_bus[sB][sB],y_selfAdmittance);
            Y_bus[eB][eB] = complex.add(Y_bus[eB][eB],y_selfAdmittance);
        }
        complex.printMatrix(Y_bus, "The bus admittance matrix is");
        
    }
    public static double[] column_extract(double a[][],int col) {
        int rows = a.length;
        int cols = a[0].length;
        double[] res = new double[rows];
        for(int i=0;i<rows;i++) {
            res[i]=a[i][col-1];
        }
        if(col>cols)
        System.out.println("Invalid column requested/matrix given ");
        return res;
    }
    public static void printArray(double a[]) {
        int length = a.length;
        for(int i=0;i<length;i++)
            System.out.println(a[i]+" ");
    }
    public static void printArray(double a[], String info) {
        System.out.println(info);
        printArray(a);
    }
    public static void printMatrix(double a[][]) {
        int rows = a.length;
        int cols = a[0].length;
        for(int i = 0;i<rows;i++) {
            for(int j = 0;j<cols;j++) {
                System.out.print(a[i][j]+" ");
            }
            System.out.println();
        }
        
    }
    public static void printMatrix(double a[][], String info) {
        System.out.println(info);
        int rows = a.length;
        int cols = a[0].length;
        for(int i = 0;i<rows;i++) {
            for(int j = 0;j<cols;j++) {
                System.out.print(a[i][j]+" ");
            }
            System.out.println();
        }
        
    }
    
    
        
    }

Program for complex functions:

package com.PS1;
import java.lang.Math;
public class complex {
    double r,i;
public complex(double its_r, double its_i) {
    this.r = its_r;
    this.i = its_i;
}
public complex(int its_r, int its_i){
    this.r = (double) its_r;
    this.i = (double) its_i;
}
public complex(int its_r,double its_i){
    this.r = (float) its_r;
    this.i =  its_i;
}
public static complex non_nullify(complex n1){
    complex n = new complex(0, 0);
    return n;
}
public static complex nullCheck(complex n1) {
    if(n1==null)
    n1 = complex.non_nullify(n1);
    return n1;
}
public static complex add(complex n1, complex n2) {
    n1=nullCheck(n1);
    n2=nullCheck(n2);
    complex sum = new complex(0,0);
    sum.r = n1.r+n2.r;
    sum.i = n1.i+n2.i;
    return sum;
}
public static complex subtract(complex n1,complex n2) {
    n1=nullCheck(n1);
    n2=nullCheck(n2);
    complex diff = new complex(0, 0);
    diff.r = n1.r-n2.r;
    diff.i = n1.i-n2.i;
    return diff;
}
public static complex mul(complex n1, complex n2) {
    n1=nullCheck(n1);
    n2=nullCheck(n2);
    complex prod = new complex(0, 0);
    prod.r = (n1.r*n2.r) - (n1.i*n2.i);
    prod.i = (n1.r*n2.i) + (n2.r*n1.i);
    return prod;
    }
public static complex div(complex n1, complex n2) {
    n1=nullCheck(n1);
    n2=nullCheck(n2);
    complex div_res = new complex(0,0);
    double denom = mod(n2);
    div_res.r = (float) (((n1.r*n2.r)+(n1.i*n2.i))/Math.pow(denom,2));
    div_res.i = (float) (((n2.r*n1.i)-(n1.r*n2.i))/Math.pow(denom,2));
    return div_res;
}
public static complex conj(complex n1) {
    n1=nullCheck(n1);
    complex conjug = new complex(0, 0);
    conjug.r = n1.r;
    conjug.i = -n1.i;
    return conjug;
    }
public static double mod(complex n1) {
    n1=nullCheck(n1);
    double modulus = Math.sqrt(Math.pow(n1.r, 2)+Math.pow(n1.i, 2));
    return modulus;
}
public static complex reciproc(complex n1) {
    n1=nullCheck(n1);
    complex conjugate = conj(n1);
    complex reciproc = new complex(0, 0);
    reciproc.r = (float) ((conjugate.r)/Math.pow(mod(n1), 2));
    reciproc.i = (float) ((conjugate.i)/Math.pow(mod(n1), 2));
    return reciproc;
}
public static void print(complex n1) {
    n1=nullCheck(n1);
    System.out.printf("%.1f + %.1fi", n1.r,n1.i);
}
public static void println(complex n1) {
    n1 = nullCheck(n1);
    System.out.printf("%.1f + %.1fi\n", n1.r,n1.i);
}
public static void print(complex n1, String info) {
    System.out.println(info);
    print(n1);
}
public static void printMatrix(complex a[][]) {
    int rows = a.length;
    int cols = a[0].length;
    System.out.print('[');
    for(int i = 0;i<rows;i++) {
        for(int j = 0;j<cols;j++) {
            complex.print(a[i][j]);
            if(j<cols-1)
            System.out.print(", ");
            else 
            System.out.print(';');
        }
        if(i<rows-1);
        else 
        System.out.print(']');
        System.out.println();
    }
    
}
public static void printMatrix(complex a[][], String info) {
    System.out.println(info);
    printMatrix(a);
    
}
public static void main(String args[]) {
    complex n1 = new complex(2, 3);
    complex n2 = new complex(3, 4);
    complex productComplex = new complex(0, 0);
    productComplex = mul(n1,n2);
    print(productComplex);
    complex divComplex = new complex(0, 0);
    divComplex = div(productComplex,n1);
    print(divComplex);
    complex reciprocal = new complex(0, 0);
    complex n3 = new complex(0, 0.2);
    reciprocal  =  reciproc(n3);
    print(reciprocal,"reciprocal");
    
}
}
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

For now, I only feel like reviewing complex. I may come back for FormBus_builtin later.

Canonical java has several naming and formatting conventions this code is not using. That will make it much harder for anyone familiar with those conventions to read and work with the code. Specifically:

  • Classes should being with a capital letter (Complex)
  • Underscores should only be used in the names of constants, not parameters or variables
  • There should be whitespace before (
  • There should be whitespace around ==, =, +, -, *, and /
  • There should be whitespace between method declarations.
  • Instance variables should each be on their own line.

A better name for this class might be ComplexNumber.

The two instance variables in this class should both be marked private final. They should not be directly accessed from outside the class, and they should not change once an instance of the class has ben created. This change would make the class immutable. Immutability is a powerful concept whose advantages are beyond the scope of this review.

r and i don't really make sense. Typically a complex number is defined as a+bi where a is the "real part" and b is the "imaginary part". These variables should be either a and b or realPart and imaginaryPart.

I'm not sure what the its prefix is trying to convey, but it is not necessary. this will distinguish between a parameter named r and an instance variable named r.

Only the first constructor is needed. ints can assign to double parameters and variables with no explicit casting required.

non_nullify does not make sense. It accepts an argument, ignores it, and returns a valid complex number 0+0i. There's no reason for an argument to be passed in, and this method is really just creating a new object. Instead, perhaps a public constant ZERO, which is then used by nullCheck would make more sense.

Even when optional, curly braces should be used. They make the code easier to read and eliminate a common class of error.

nullCheck could be rewritten to use the ternary operator to be return (n1 == null) ? ZERO : n1; or return (n1 == null) ? new ComplexNumber(0, 0) : n1;

The name nullCheck is very misleading. The code is not checking for null. It's replacing nulls with zeros.

The concept behind the nullCheck method is atypical. I would expect a NullPointerException if an operation is attempted on a null value. Failing fast means the code is not trying to work on bad data, returning results which are confusing and invalid.

Rather than a collection of static methods, Object-Oriented Programming proposes that objects have methods on them which allow operations on the objects themselves. Combined with immutability, this suggests code that looks more like ComplexNumber sum = cn1.plus(cn2); rather than ComplexNumber sum = ComplexNumber.add(cn1, cn2);.

Reassigning parameters is frowned upon because it adds cognitive load to the readers of code.

To make this class immutable, add, subtract, mul, and div should not create empty ComplexNumber instances and then assign their parts, but rather construct a complete ComplexNumber from precomputed parts.

Names like mul and div are hard to read. Readability is a key part of any well-written code, and should be favored over most other concerns without a good reason. multiply and divide would be highly preferable.

Reciprocal is computing the square of the modulus twice. That could be stored in a local variable. A micro-optimization, to be sure, but modulusSquared is also easier to read than Math.pow(mod(n1), 2).

There are several methods dedicated to printing out a value. This becomes much less useful as soon as somebody would like to compute a displayable value but put it anywhere other than System.out, such as in an exception, in a database, as part of a larger message, etc. It is preferable to instead override toString to return a String representation of the object, which clients can then use as they see fit.

toString would replace the first instance of print and println. The second instance of print should instead be handled by clients outputting as they see fit.

The two printMatrix calls do not belong in this class. They are unrelated to the idea of a complex number. They probably belong as a toString implementation of a class named Matrix, which it looks like could be derived from some of the code in FormBus_builtin.

Proper testing should be done using a testing framework, such as Junit. Learning that framework may be beyond the scope of this exercise, but it's worth knowing what is out there.

There are a couple of other gotchas that are not addressed which may not be relevant. One is the idea of Overflow and Underflow. double has a smallest possible value and a largest possible value. Operations which try to go lower or higher than those values will have very unexpected results. The other is that not all possible numbers can be represented by a double, so precision will be lost over time. If either of these is a concern, change Complex to use BigDecimal values instead of doubles. BigDecimal is infinitely precise, does not overflow or underflow, and can be slower than using double. BigDecimal can be used under the hood, or the constructor can be changed to accept BigDecimal instead of or in addition to double. In that case, the code will need to check for null BigDecimal arguments at creation time.

If you made all these changes (except BigDecimal), your code might look more like:

import java.lang.Math;

public final class ComplexNumber {
    
    public static final ComplexNumber ZERO = new ComplexNumber(0, 0);
    
    private double realPart;
    private double imaginaryPart;
    
    public ComplexNumber(double realPart, double imaginaryPart) {
        this.realPart = realPart;
        this.imaginaryPart = imaginaryPart;
    }
    
    public ComplexNumber plus(ComplexNumber number) {
        return new ComplexNumber(this.realPart + number.realPart, this.imaginaryPart + number.imaginaryPart);
    }
    
    public ComplexNumber minus(ComplexNumber number) {
        return new ComplexNumber(this.realPart - number.realPart, this.imaginaryPart - number.imaginaryPart);
    }
    
    public ComplexNumber times(ComplexNumber number) {
        return new ComplexNumber(this.realPart * number.realPart, this.imaginaryPart * number.imaginaryPart);
    }
    
    public ComplexNumber dividedBy(ComplexNumber number) {
        return new ComplexNumber(this.realPart / number.realPart, this.imaginaryPart / number.imaginaryPart);
    }
    
    public ComplexNumber conjugate() {
        return new ComplexNumber(this.realPart, -1 * this.imaginaryPart);
    }
    
    public double modulus() {
        return Math.sqrt(Math.pow(this.realPart, 2) + Math.pow(this.imaginaryPart, 2));
    }
    
    public ComplexNumber reciprocal() {
        double modulusSquared = Math.pow(this.modulus(), 2);
        ComplexNumber conjugate = this.conjugate();
        return new ComplexNumber(
                conjugate.realPart / modulusSquared,
                conjugate.imaginaryPart / modulusSquared);
    }
    
    @Override
    public String toString() {
        return String.format("%.1f + %.1fi",this.realPart, this.imaginaryPart);
    }
    
    public static void main(String args[]) {
        ComplexNumber n1 = new ComplexNumber(2, 3);
        ComplexNumber n2 = new ComplexNumber(3, 4);
        ComplexNumber product = n1.times(n2);
        System.out.println("Product: " + product);
        System.out.println("Quotient: " + product.dividedBy(n1));
        
        ComplexNumber n3 = new ComplexNumber(0, 0.2);
        System.out.println("Reciprocal: " + n3.reciprocal());
    }
}
\$\endgroup\$
2
  • \$\begingroup\$ One argument in favour of add, sub, mul, and div as function names would be that they are all short and the same length, making arithmetic-heavy code easier to read. \$\endgroup\$ Mar 6, 2023 at 16:41
  • \$\begingroup\$ the multiplication and division functions are meant for complex numbers and the math is kinda different. I found this program online that has the exact formula, implemented the way you suggested, I guess. [link] (algs4.cs.princeton.edu/code/edu/princeton/cs/algs4/…) \$\endgroup\$
    – Aloy
    Mar 13, 2023 at 16:12

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.