3
\$\begingroup\$

I wrote a complex number class using JS for practice.

class Complex {

    constructor(real,imag) {
        this.real = real;
        this.imag = imag;
    }

    getReal() {
        return this.real;
    }

    getImag() {
        return this.imag;
    }

    setReal(real) {
        this.real = real;
    }

    setImag(imag) {
        this.imag = imag;
    }

    static add(a,b) {
        return new Complex(a.real + b.real, a.imag + b.imag);
    }

    static subtract(a,b) {
        return new Complex(a.real - b.real, a.imag - b.imag);
    }

}

const a = new Complex(1,0);
const b = new Complex(5,10);
console.log("real: ",a.getReal());
console.log("imag: ",a.getImag());
let c = Complex.add(a,b);
console.log("add: ",c);
c = Complex.subtract(a,b);
console.log("subtract: ",c);

What would you recommend to me, to make it better.

\$\endgroup\$

2 Answers 2

4
\$\begingroup\$

Some minor recommendations:

  1. Make it immutable.

It's generally a good thing to design for immutability when it's not overly complicated to do so. Take this code snippet for example:

const x = 2
fn(x)
console.log(x) // 2

It's going to output 2. You know it will because numbers are immutable, so there's nothing that fn() can do to change the value of that number. However, the same can't be said for this piece of logic, because your class is mutable:

const x = new Complex(2, 0);
fn(x);
console.log(x); // ??

fn() could be altering the value of x, and there's no way for you to know if it will or not without looking at the definition of fn().

Here's how I might go about making it immutable and still easy to work with:

class Complex {
    constructor(real,imag) {
        this.real = real;
        this.imag = imag;
        Object.freeze(this); // See here: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/freeze
    }

    withRealPart(newReal) {
        return new Complex(newReal, this.imag);
    }

    withImaginaryPart(newImaginary) {
        return new Complex(this.real, newImaginary);
    }

    // Remove the setReal() and setImag() functions

    ...
}

Though, the withRealPart() and withImaginaryPart() functions might not even be that necessary. If you don't find yourself using them a lot, just get rid of them. It's probably more common to create an entirely new imaginary number or to add/subtract them, than to replace a single "part" with a new value.

  1. Toss the useless getters

They weren't doing anything anyways. You had publicly exposed the "real" and "imag" properties, letting anyone directly see or alter those public properties without going through your getters or setters, so why would they bother using those functions? Even if you had made those properties private, I would have suggested that you just make them public and toss the getters/setters, they're just noise in this scenario. (you may find some purist OOP people who will disagree with me on this, so feel free to look around and make your own opinions on this matter).

  1. Improve naming

This point doesn't matter that much, but I would recommend just spelling out the full "imaginary" word in your code. It doesn't take up that much more room, and is a little easier to read (and, you should in general optimize for reading, not writing). Perhaps, consider calling your class ComplexNumber as well, to make it more apparent what it's for. (I originally thought, from this question's title, that you were literally trying to make an arbitrarily complex class full of complex logic. It wasn't until I looked at your code snippet that I realized you were just trying to work with complex numbers).

\$\endgroup\$
2
  • 2
    \$\begingroup\$ I agree with tossing the useless getters/setters. If it turns out that you later need to retrofit getters/setters, you can do it using the ECMAScript 6 syntax: get/set. \$\endgroup\$ Feb 25 at 7:07
  • \$\begingroup\$ Uh thanks a lot \$\endgroup\$
    – laki
    Feb 25 at 16:21
1
\$\begingroup\$

A short review;

  • all the points by @Scotty Jamison are super valid
  • I would suggest a toString() function that provides a string representation of the imaginary number
  • I would call your add function sum, and let the add function add to the instance
  • I think using i instead of imag would work too.

class ComplexNumber {

    constructor(real, imaginary) {
        this.real = real;
        this.imaginary = imaginary;
    }

    add(complexNumber) {
        return new ComplexNumber(this.real + complexNumber.real, this.imaginary + complexNumber.imaginary);
    }
    
    subtract(complexNumber) {
        return new ComplexNumber(this.real - complexNumber.real, this.imaginary - complexNumber.imaginary);
    }
    
    toString(){
      const real = (this.real?this.real:"")
      const sign = (this.real>0?"+":"");
      const imaginary = (this.imaginary?this.imaginary+"i":"")
      return real + sign + imaginary;
    }

    static sum(a, b) {
        return new Complex(a.add(b));
    }
}

const a = new ComplexNumber(2,3);
const b = new ComplexNumber(0,1);
const sum = a.add(b);
const neg = b.subtract(a);
console.log(a.toString());
console.log(b.toString());
console.log(sum.toString());
console.log(neg.toString());

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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