5

I have a Runnable class like:

Class R1 implements Runnable {
  private static final Log LOGGER = LogFactory.getLog(R1.class);
  private final ObjectClass obj;
  private final SomeService service;

  public R1(ObjectClass obj, SomeService service) {
     this.obj = obj;
     this.service = service;
  }

  @override
  public void run() {
    String value = this.obj.getSomeValue();
    LOGGER.debug("Value is " + value);
    // some actions, such as:
    // service.someMethod(obj);
  }
}

I use a ExecutorService object to execute R1 and put R1 in a queue. But later outside R1 I change the value in the ObjectClass that I passed in R1 so the the actions in R1 after getSomeValue() aren't behaving as I expected. If I want to keep the value of ObjectClass object in R1 unchanged what can I do? Suppose the object is big and has a lot of get and set methods.

To make the problem clearer, I need to pass the obj into a service class object which is also used as a parameter in the runnable class. I have changed the original codes accordingly.

newguy
  • 5,668
  • 12
  • 55
  • 95

7 Answers7

1

As per comments, apparently my suggested solution has problems.

As such, follow the other suggestions about creating a new instance and copying the properties you require across. Or create a lightweight data object that holds the properties you require. Either way, I believe you need 2 instances to do what you want.


I suggest you could implement clone method that creates a new instance.

http://download.oracle.com/javase/1,5,0/docs/api/java/lang/Cloneable.html

The problem here is that you have passed the instance into your R1class, but it is still the same single instance, so changes to it will affect everything else. So, implementing a clone method will allow you to easily create a copy of your instance that can be used in your R1 class, while allowing you to make further changes to your original.

In your R1 class,

public R1(ObjectClass obj) {
   //this.obj = obj;
   this.obj = obj.clone();
}

P.S. you must implement this method yourself. It won't just automatically give you a deep copy.

Daryl Teo
  • 5,394
  • 1
  • 31
  • 37
  • That won't help - you'll still get the same problem. It's a synchronization/publishing issue. – Bohemian Sep 20 '11 at 03:15
  • 1
    Cloning usually isn't suggested as part of a solution. It's usually part of a problem. See for example http://stackoverflow.com/questions/709380/java-rationale-of-the-cloneable-interface – Ryan Stewart Sep 20 '11 at 03:17
  • @Bohemian I disagree. He states that R1 is placed in a Queue before execution, and between instantiation and execution, he modifies the R1 object properties, which gives him a different result when it gets run. He also seems adamant on storing the entire object, which is why I suggested making a copy so that he has 2 instances, not 1, so that changes to the first instance won't affect the second. Unless, this is some special functionality specific to executorService (which I have not used before), then I apologise for my oversight. – Daryl Teo Sep 20 '11 at 03:35
  • @RyanStewart I was not aware of issues with Cloneable. But for this situation, even if he doesn't implement cloneable, I believe his solution calls for 2 instances of his object... whether he achieves this via copy, or via another instantiaton and copying of properties. Upvote anyway :) – Daryl Teo Sep 20 '11 at 03:37
  • I end up using a home made clone class that does the deep clone job. – newguy Sep 22 '11 at 01:32
1

Depending on the nature of your program, there are a couple options.

You could "Override Clone Judiciously" (Item 11 in Effective Java) and clone the object before handing it to the runnable. If overriding clone doesn't work for you, it might be better to do one of the following:

  1. Create a new instance of the object manually and copy the values from obj.
  2. Add a subset of the data contained in obj. So instead of passing obj into the constructor, you would pass in someValue. I would advocate this method, so that you only supply R1 with the data it needs, and not the entire object.

Alternatively, if it doesn't matter that the data in obj changes before R1 is executed, then you only need to make sure that obj doesn't change while R1 is executing. In this case, you could add the synchronize keyword to the getSomeValue() method, and then have R1 synchronize on obj like so:

@Override
public void run() {
  synchronize (obj) {
    String value = obj.getSomeValue();
  }
  // some actions.
}
Neil Traft
  • 18,367
  • 15
  • 63
  • 70
0

Pass the object to the constructor, and don't keep a reference to it.

Ryan Stewart
  • 126,015
  • 21
  • 180
  • 199
0

if objet is too big,

maybe an immutable ParameterObject, with enough data/method ,is better.

swanliu
  • 781
  • 3
  • 8
0

If possible, try making your ObjectClass immutable. (no state changes supported). In Java you have to "do this yourself"; there's no notion of 'const' object (as in C++)

Perhaps you can have your orig ObjectClass but create a new class ImmutableObjectClass which takes your orig in the ctor.

seand
  • 5,168
  • 1
  • 24
  • 37
0

Assumption: You don't care if R1 operates on old data.

You can then change your code to:

public class R1 implements Runnable {
  private final String value;

  // Option 1: Pull out the String in the constructor.
  public R1(ObjectClass obj) {
    this.value = obj.getSomeValue(); // Now it is immutable
  }

  // Option 2: Pass the String directly into the constructor.
  public R1(String value) {
    this.value = value; // This constructor has no coupling
  }

  @Override public void run() {
    // Do stuff with value
  }
}

If you want R1 to operate on the latest data, as opposed to what the data was when you constructed it, then you will need some type of synchronisation between R1 and the data modification.

0

The "problem" here is that under the Java Memory Model, threads may (and do) cache field values. This means that if one thread updates a field (of the ObjectClass object), other threads won't "see" the change - they'll still be looking at their cached (stale) value.

In order to make change visible across threads you have two options:

  1. Make the fields you'll be changing in ObjectClass volatile - the volatile keyword forces threads to not cache the field's value (ie always use the latest value)
  2. synchronize access, both read and write, to the fields - all changes made within a synchronized block are visible to other threads synchronizing on the same lock object (if you synchronize methods, the this object is used as the lock)
Bohemian
  • 412,405
  • 93
  • 575
  • 722