Skip to main content
Commonmark migration
Source Link

##What I'd suggest?

What I'd suggest?

##What I'd suggest?

What I'd suggest?

Source Link
maaartinus
  • 13.5k
  • 1
  • 34
  • 73

The code as given does not compile as getCopy, getCoefficient, getDegree, and isVariable are missing from class Monomial.

Some toString method should also be defined, otherwise debugging is a real pain. You're also missing equals and hashCode.

@post for every v, 'a'<=v<='z', isVariable(v) == false

Should this explain or obfuscate something?

this.ceof=coefficient;

Usually, exactly the same name gets used in constructors and setters. And that's why you need the this qualifier.

Creates a polynomial with (safe copies of) the given monomials

Sometimes, copying may be more (or less) efficient, but generally, immutability is the way to go. With an immutable Monomial you don't have to hunt bugs caused by forgotten calls to getCopy.

public Polynomial(Monomial[] monomials) {
    this.monoms=monomials;
}

What's worse... the code contradicts the comment.

public Monomial getMonomial(int index)

A polynomial is not an indexed array of monomials. Actually, there's no order, so I'd avoid any public methods giving such an access. This may or may not be possible.

public Polynomial add(Polynomial other) {

You should group the monomials here. What you do is representing (x)+(x) as two monomials instead of 2*x.

Monomial[] result=new Monomial[this.getMonomialCount()*other.getMonomialCount()];

This makes it pretty unusable. From something as simple as (x+y)*(x-y) you get x*x - x*y + x*y - y*y instead of x*x - y*y. With every operation, your polynomial grows, never to shrink again.

if(thisMonomial.isVariable((char)('a'+k))==true&&otherMonomial.isVariable((char)('a'+k))==true){
                newDegree=thisMonomial.getDegree((char)('a'+k))+otherMonomial.getDegree((char)('a'+k));
                }
                else{
                    if(thisMonomial.isVariable((char)('a'+k))==true){
                        newDegree=thisMonomial.getDegree((char)('a'+k));
                    }
                    else{
                        newDegree=otherMonomial.getDegree((char)('a'+k));
                    }
                    }

You're needlessly special-casing degree 0, which blows your expression to something terribly complicated... and also broken: In case of both degrees being 0, you simply keep newDegree from the previous iteration as you missed this case. Always minimize the scope. Simply using

char v = (char)('a'+k);
int newDegree = thisMonomial.getDegree(v) + otherMonomial.getDegree(v);

would do.

##What I'd suggest?

Create an immutable class UnitMonomial (monomial with coefficient equal 1) with a method multiply(UnitMonomial) and represent Polynomial as Map<UnitMonomial, Integer>. This allows you to simplify the results of all operations.