«

Hello all,

I have a simple container object that has a get and set method for a
variable it contains. Multiple threads will potentially be accessing
the container object to get and set the variable. Can anyone tell me
what's wrong with the code I have below? It's not working for me...

public class Container {

private boolean value = false;
private boolean available = false;

public Container(boolean value) {
this.value = value;
}

public synchronized boolean get() {
while (available == false) {
try {
wait();
} catch (InterruptedException ex) { ex.printStackTrace(); }
}
available = false;
notifyAll();
return value;
}

public synchronized void set(boolean value) {
while (available == true) {
try {
wait();
} catch (InterruptedException ex) { ex.printStackTrace(); }
}
this.value = value;
available = true;
notifyAll();
}
}

I found most of the above code in a tutorial on the web --
http://www.janeg.ca/scjp/threads/synchronized.html -- but like I said
it's not working for me. Here's how I'm testing it:

I have a class that extends TimerTask and is called every 5 seconds to
randomize the value by calling the set method and passing it a random
boolean value obtained from Random.nextBoolean(). I have it set up to
print to the console each time it changes the value... it prints one
time but never prints again. Any suggestions as to why this isn't
working for me?

Thanks!

Bryan wrote:
> Hello all,
>
> I have a simple container object that has a get and set method for a
> variable it contains. Multiple threads will potentially be accessing
> the container object to get and set the variable. Can anyone tell me
> what's wrong with the code I have below? It's not working for me...
>
> public class Container {
>
> private boolean value = false;
> private boolean available = false;
>
> public Container(boolean value) {
> this.value = value;
> }
>
> public synchronized boolean get() {
> while (available == false) {
> try {
> wait();
> } catch (InterruptedException ex) { ex.printStackTrace(); }
> }
> available = false;
> notifyAll();
> return value;
> }
>
> public synchronized void set(boolean value) {
> while (available == true) {
> try {
> wait();
> } catch (InterruptedException ex) { ex.printStackTrace(); }
> }
> this.value = value;
> available = true;
> notifyAll();
> }
> }
>
> I found most of the above code in a tutorial on the web --
> http://www.janeg.ca/scjp/threads/synchronized.html -- but like I said
> it's not working for me. Here's how I'm testing it:
>
> I have a class that extends TimerTask and is called every 5 seconds to
> randomize the value by calling the set method and passing it a random
> boolean value obtained from Random.nextBoolean(). I have it set up to
> print to the console each time it changes the value... it prints one
> time but never prints again. Any suggestions as to why this isn't
> working for me?
>
> Thanks!

> it's not working for me
'It' is your code, not the tutorial code which is too simple to have
any problem.
Post a small demo code that is generally compilable, runnable and could
reproduce your problem. See:
http://homepage1.nifty.com/algafield/sscce.html and
http://www.yoda.arachsys.com/java/newsgroups.html

Bryan wrote:

> I have a class that extends TimerTask and is called every 5 seconds to
You should use multiple threads accessing your single Container object.
Otherwise, you can't test the synchronized/wait/notify mechanism.

'Container' is a bad naming.

Bryan wrote:
> Hello all,
>
> I have a simple container object that has a get and set method for a
> variable it contains. Multiple threads will potentially be accessing
> the container object to get and set the variable. Can anyone tell me
> what's wrong with the code I have below? It's not working for me...
>
> public class Container {
>
> private boolean value = false;
> private boolean available = false;
>
> public Container(boolean value) {
> this.value = value;
> }
>
> public synchronized boolean get() {
> while (available == false) {
> try {
> wait();
> } catch (InterruptedException ex) { ex.printStackTrace(); }
> }
> available = false;
> notifyAll();
> return value;
> }
>
> public synchronized void set(boolean value) {
> while (available == true) {
> try {
> wait();
> } catch (InterruptedException ex) { ex.printStackTrace(); }
> }
> this.value = value;
> available = true;
> notifyAll();
> }
> }
>
> I found most of the above code in a tutorial on the web --
> http://www.janeg.ca/scjp/threads/synchronized.html -- but like I said
> it's not working for me. Here's how I'm testing it:
>
> I have a class that extends TimerTask and is called every 5 seconds to
> randomize the value by calling the set method and passing it a random
> boolean value obtained from Random.nextBoolean(). I have it set up to
> print to the console each time it changes the value... it prints one
> time but never prints again. Any suggestions as to why this isn't
> working for me?
>
> Thanks!
>
Hmmm... the example at the website looks ok, but i wonder whether you
misunderstood its purpose, i.e. how the consumer-producer scenario works.

The idea is that you _cannot_ set ("produce") a new value _before_ the
old is consumed. That means to call set for a 2nd time and having an
effect, you need to call get() before, which consumes the value and lets
you set/produce another later.
The consumer-producer scenario is to avoid race conditions and to make
sure no information gets overwritten by accident (a value that is
replaced by another via a set() call before it was consumed by a get() call.
This is a typical scenario for a multithreaded buffer, where you want to
avoid that its content gets overwritten by incoming new data before it
was read, since that would mean data loss

Ah yes... I did in fact misunderstand the purpose. Thanks for clearing
that up. I simply want to make sure that one thread doesn't try to
read at the same time another thread is writing. If so, I want the
read to come after the write. Otherwise, I'm not worried about data
loss.

Thanks!

> Hmmm... the example at the website looks ok, but i wonder whether you
> misunderstood its purpose, i.e. how the consumer-producer scenario works.
>
> The idea is that you _cannot_ set ("produce") a new value _before_ the
> old is consumed. That means to call set for a 2nd time and having an
> effect, you need to call get() before, which consumes the value and lets
> you set/produce another later.
> The consumer-producer scenario is to avoid race conditions and to make
> sure no information gets overwritten by accident (a value that is
> replaced by another via a set() call before it was consumed by a get() call.
> This is a typical scenario for a multithreaded buffer, where you want to
> avoid that its content gets overwritten by incoming new data before it
> was read, since that would mean data loss

On Dec 17, 4:51 pm, "Bryan" <BTRichard...@gmail.com> wrote:
> Ah yes... I did in fact misunderstand the purpose. Thanks for clearing
> that up. I simply want to make sure that one thread doesn't try to
> read at the same time another thread is writing. If so, I want the
> read to come after the write. Otherwise, I'm not worried about data
> loss.
>
> Thanks!
>
> > Hmmm... the example at the website looks ok, but i wonder whether you
> > misunderstood its purpose, i.e. how the consumer-producer scenario works.
>
> > The idea is that you _cannot_ set ("produce") a new value _before_ the
> > old is consumed. That means to call set for a 2nd time and having an
> > effect, you need to call get() before, which consumes the value and lets
> > you set/produce another later.
> > The consumer-producer scenario is to avoid race conditions and to make
> > sure no information gets overwritten by accident (a value that is
> > replaced by another via a set() call before it was consumed by a get() call.
> > This is a typical scenario for a multithreaded buffer, where you want to
> > avoid that its content gets overwritten by incoming new data before it
> > was read, since that would mean data loss


If thats all you want to do,then you have a lot less choices...

1) Don't use objects whose values can be changed - use multiple
'immutable' objects. So the first thread uses the first object, then
the second thread one uses a different object based upon the value that
correct for it. (sounds compilcated and it is in some ways, but you
get great performance.

2) - The one which sounds like the best for your case. - Just
synchronize your methods like you have, without all of the extra
gubbings.

e.g.

public class Container {

private boolean value = false;

public Container(boolean value) {
this.value = value;
}

public synchronized boolean get() {
return value;
}

public synchronized void set(boolean value) {
this.value = value;
}

}

> public class Container {
>
> private boolean value = false; // redundant assignment
>
> public Container(boolean value) {
> this.value = value;
> }
>
> public synchronized boolean get() {
> return value;
> }
>
> public synchronized void set(boolean value) {
> this.value = value;
> }
>
> }

Looks like a mutable, synchronized Boolean. Might want to consider a class
name like SynchronizedBoolean. Might want to rename the get()/set() methods,
either to follow JavaBean conventions or to mimic similar methods in
java.lang.Boolean.

- Lew

Lew wrote:
>> public class Container {
>>
>> private boolean value = false; // redundant assignment
>>
>> public Container(boolean value) {
>> this.value = value;
>> }
>>
>> public synchronized boolean get() {
>> return value;
>> }
>>
>> public synchronized void set(boolean value) {
>> this.value = value;
>> }
>>
>> }
>
>
> Looks like a mutable, synchronized Boolean. Might want to consider a
> class name like SynchronizedBoolean. Might want to rename the
> get()/set() methods, either to follow JavaBean conventions or to mimic
> similar methods in java.lang.Boolean.
>
> - Lew

Why not

public class Container<Foo> {
private Foo contents;
public Container (Foo initialContents) {
contents = initialContents;
}
public synchronized Foo getContents () {
return contents;
}
public synchronized setContents (Foo newContents) {
contents = newContents;
}
}

Container<Foo> fooHolder = new Container<Foo>(new Boolean(false));

It's reusable for holding any object now.

John Ersatznom wrote:
> Lew wrote:
>
>>> public class Container {
>>>
>>> private boolean value = false; // redundant assignment
>>>
>>> public Container(boolean value) {
>>> this.value = value;
>>> }
>>>
>>> public synchronized boolean get() {
>>> return value;
>>> }
>>>
>>> public synchronized void set(boolean value) {
>>> this.value = value;
>>> }
>>>
>>> }
>>
>>
>>
>> Looks like a mutable, synchronized Boolean. Might want to consider a
>> class name like SynchronizedBoolean. Might want to rename the
>> get()/set() methods, either to follow JavaBean conventions or to mimic
>> similar methods in java.lang.Boolean.
>>
>> - Lew
>
>
> Why not
>
> public class Container<Foo> {
> private Foo contents;
> public Container (Foo initialContents) {
> contents = initialContents;
> }
> public synchronized Foo getContents () {
> return contents;
> }
> public synchronized setContents (Foo newContents) {
> contents = newContents;
> }
> }
>
> Container<Foo> fooHolder = new Container<Foo>(new Boolean(false));

Meh, make that last

Container<Boolean> booleanHolder = new Container<Boolean>(new
Boolean(false));

Thanks for the comments guys. Looks like I'll go with synchronizing
the get/set methods!

On Dec 17, 12:44 pm, John Ersatznom <j.ers...@nowhere.invalid> wrote:
> John Ersatznom wrote:
> > Lew wrote:
>
> >>> public class Container {
>
> >>> private boolean value = false; // redundant assignment
>
> >>> public Container(boolean value) {
> >>> this.value = value;
> >>> }
>
> >>> public synchronized boolean get() {
> >>> return value;
> >>> }
>
> >>> public synchronized void set(boolean value) {
> >>> this.value = value;
> >>> }
>
> >>> }
>
> >> Looks like a mutable, synchronized Boolean. Might want to consider a
> >> class name like SynchronizedBoolean. Might want to rename the
> >> get()/set() methods, either to follow JavaBean conventions or to mimic
> >> similar methods in java.lang.Boolean.
>
> >> - Lew
>
> > Why not
>
> > public class Container<Foo> {
> > private Foo contents;
> > public Container (Foo initialContents) {
> > contents = initialContents;
> > }
> > public synchronized Foo getContents () {
> > return contents;
> > }
> > public synchronized setContents (Foo newContents) {
> > contents = newContents;
> > }
> > }
>
> > Container<Foo> fooHolder = new Container<Foo>(new Boolean(false));Meh, make that last
>
> Container<Boolean> booleanHolder = new Container<Boolean>(new
> Boolean(false));