1

I have code that looks like this:

public class Polynomial {
    List<Term> term = new LinkedList<Term>();

and it seems that whenever I do something like term.add(anotherTerm), with anotherTerm being... another Term object, it seems anotherTerm is referencing the same thing as what I've just inserted into term so that whenever I try to change anotherTerm, term.get(2) (let's say) get's changed too.

How can I prevent this from happening?

Since code was requested:

//since I was lazy and didn't want to go through the extra step of Polynomial.term.add
public void insert(Term inserting) {
    term.add(inserting);
}

Code calling the insert method:

poly.insert(anotherTerm);

Code creating the anotherTerm Term:

Term anotherTerm = new Term(3, 7.6); //sets coefficient and power to 3 and 7.6

New code calling the insert method:

poly.insert((Term)anotherTerm.clone());

Which unfortunately still doesn't work due to clone() has protected access in java.lang.Object, even after doing public class Term implements Cloneable{

wrongusername
  • 18,564
  • 40
  • 130
  • 214
  • 4
    Please post the code when you're doing the instantiation and inserting. It sounds a lot like you're not creating your objects properly... Edit: Can you show us the call to insert (if this is where you create your object)? – Mia Clarke May 06 '10 at 06:57
  • What do you mean when you say you "change anotherTerm"? Do you mean you're changing (say) the coefficient from 3 to 4? Or do you mean something else? –  May 06 '10 at 07:14
  • @Willie Wheeler--yes, that's exactly what I meant. Sorry for the ambiguity – wrongusername May 06 '10 at 07:14
  • OK, and so when you change the coefficient in one of the terms, you're saying that the coefficient in other terms changes to the new value as well? In all the other terms? Just one? –  May 06 '10 at 07:16
  • Just the coefficient of the one term in the LinkedList changes – wrongusername May 06 '10 at 07:17
  • Oh, gotcha... I think, anyway, if I understand what you are saying. The term in the linked list is the same as the term outside of the linked list. So when you change one you change the other... Ah, which is what you're saying. I'll post a proper answer. One sec. –  May 06 '10 at 07:20

5 Answers5

4

The solution is simple: make Term immutable.

Effective Java 2nd Edition, Item 15: Minimize mutability:

  • Immutable objects are simple.
  • Immutable objects can be shared freely.
  • Immutable objects make great building blocks for other objects.
  • Classes should be immutable unless there's a very good reason to make them mutable.
  • If a class cannot be made immutable, limit its mutability as much as possible.
    • Make every field final unless there is a compelling reason to make it non-final

Something as simple and small as Term really should be made immutable. It's a much better overall design, and you wouldn't have to worry about things like you were asking in your question.

See also


This advice becomes even more compelling since the other answers are suggesting that you use clone().

Effective Java 2nd Edition, Item 11: Override clone judiciously

Because of the many shortcomings, some expert programmers simply choose to never override the clone method and never invoke it except, perhaps, to copy arrays.

From an interview with author Josh Bloch:

If you've read the item about cloning in my book, especially if you read between the lines, you will know that I think clone is deeply broken.

DO NOT make Term implements Cloneable. Make it immutable instead.

See also

Community
  • 1
  • 1
polygenelubricants
  • 376,812
  • 128
  • 561
  • 623
  • @polygenelubricants--Thanks, I'll try looking into that! – wrongusername May 06 '10 at 07:16
  • @polygenelubricants--true, but I make changes to `Term`s in my program... it'll require a few tweaks :) – wrongusername May 06 '10 at 07:32
  • I don't disagree with the suggestion (a Term class is the sort of thing that one would expect to see immutable), but note that as stated he seems to want to be able to change the values. If he's able to give that requirement up (replacing it, for example, with the alternative of just creating a new Term) then immutability works. Otherwise no. –  May 06 '10 at 07:33
  • @wrongusername: create new `Term` objects for every value, just like `String`, `BigInteger`, and all other immutable classes, which are much more complex and can get much larger than a simple 2-number `Term`. Don't worry about performance and speed. Better design is what you should strive for. It will pay off greatly in the end. – polygenelubricants May 06 '10 at 07:34
  • @wrongusername: Yeah, you beat me to your own answer. :-) It is semi-easy to imagine situations in which you wouldn't want to make Term immutable. Maybe the polynomial is modeling some rapidly and dynamically changing phenomenon and you don't want to have to create new Terms every time a coefficient changes. Etc. –  May 06 '10 at 07:35
  • @poly: I don't think you can dismiss mutability so easily. Consider, e.g., AWT classes like Point, Rectangle, Dimension, etc. These are similar sorts of class (coordinate container basically). If you're rendering lots of stuff you wouldn't want a bunch of garbage collection happening all over the place... –  May 06 '10 at 07:39
  • @Willie: JVM is pretty good at handling this usage pattern nowadays. That's what I hear anyway. You can create millions of one-time-use small objects, and the JVM will take care of it just fine. You don't want to foresake good design principles because you're worried how the JVM will handle it. If the principle is truly that good, then the JVM will have to catch up and accommodate. – polygenelubricants May 06 '10 at 07:42
  • @Willie--Performance isn't really what I'm aiming for here, so I guess I'll go along with his suggestion and see what I can do :) – wrongusername May 06 '10 at 07:44
  • @poly: Yeah, that is correct generally speaking. The various object pooling approaches from the days of old have mostly been discredited except in cases where the object creation is expensive (e.g. Connections, Threads, etc.). And realistically I expect your advice here is correct. But it's still a numbers game. In rendering an animated particle cloud I don't think creating new Points is going to be a correct approach. But yes, as Bloch says, prefer immutability for sure. If you can do it, do it. –  May 06 '10 at 07:49
  • @polygenelubricants--by the way, what advantage does setting `Term` immutable offer as compared to `new Term(coefficient, power)` as a workaround if you're going have to do that every time with a final Term anyways? – wrongusername May 06 '10 at 07:51
  • @wrongusername: the advantage is that you won't have to worry about behind-your-back mutation through shared references ever again. As long as `Term` is mutable, you'd have to always use the "workaround" everywhere you're worried about shared references. Once `Term` is immutable, you stop worrying forever. – polygenelubricants May 06 '10 at 07:54
2

OK, replacing my old answer with this, now that I understand the question and behavior better.

You can do this if you like:

public void insertTerm(Term term) {
    polynomial.insert(new Term(term));
}

and then create a new Term constructor like this:

public Term(Term term) {
    this.coefficient = term.coefficient;
    this.exponent = term.exponent;
}

That should work.

  • @Willie Wheeler--yeah, that wasn't originally in my code until the answers popped up, so I removed it. But I get syntax errors trying to clone the Term so I can't even try the code out :( – wrongusername May 06 '10 at 07:10
  • Thank you! It did work :) And thank you all for spending this time figuring the problem out – wrongusername May 06 '10 at 07:31
2

EDIT: Ok, I think I see what it is you're doing now. If you have this class:

public class Polynomial 
{
    List<Term> term = new LinkedList<Term>();

    public void insert(Term inserting) 
    {
       term.add(inserting);
    }
}

And then you do this:

Polynomal poly = new Polynomal()
Term term = new Term();
poly.insert(term);
term.coefficient = 4;

...then the object term is the same object as poly.get(0). "term" and "poly.get(0)" are both references to the same object - changing one will change the other.

Mia Clarke
  • 8,134
  • 3
  • 49
  • 62
  • I don't think that's what I'm doing... basically, if the LinkedList is empty and I try term.add(anotherTerm), and I change anotherTerm's coefficient to, say, 4, term.get(0).coefficient would be changed to 4 as well – wrongusername May 06 '10 at 07:19
1

Question is no so clear, but i just try , when you are adding the objects , add anotherTerm.clone()

srinannapa
  • 3,085
  • 8
  • 49
  • 66
  • You need to implement Clonable , class Term implements Clonable – srinannapa May 06 '10 at 07:05
  • @srinannapa--cannot find symbol. symbol: class Clonable :( – wrongusername May 06 '10 at 07:07
  • wrongusername: Clonable is an interface. You need to write "implements Cloneable" rather than "extends Cloneable" after your class definition. However, you shouldn't have to clone your objects, just instantiating a new one should do the trick. – Mia Clarke May 06 '10 at 07:08
1

It sounds like you are not instantiating new Objects, just referencing the same one. You should instantiate a new Term, either with Term term = new Term(); or by cloning term.clone().

EDIT to be able to be cloned, Term need to implement the Cloneable interface. That means that you are responsible for how the new copy of a Term should be defined.

Hard to tell without seeing the code that calls the insert method, but sounds like that is the problem.

Lars Andren
  • 8,601
  • 7
  • 41
  • 56
  • How can I clone the Term? Something like `poly.insert((Term)anotherTerm.clone());` (which doesn't work)? I think it doesn't work because it "has protected access in java.lang.Object" – wrongusername May 06 '10 at 07:03