Hi all, ain't been here in a while, busy busy busy with college...

I have an assignment on threads for Operating Systems, I can't actually see how it relates to OS but it is thread based, I haven't worked with threads before, but I have it finished to the best of my ability, I'm not looking for someone to do it for me... it's done, I'm just wondering if anyone can see anywhere I can improve it... Here it is:

THE BRIEF

Take a copy of the Java source code in Assignment01.java. It implements four classes: Bag, Parent, Child and Sweets. Sweets creates an instance of Bag which is shared by the two threads, Parent and Child. The Parent repeatedly puts sweets into the bag while the Child repeatedly takes sweets out of the bag. As it stands the code will run but produces some rather strange results. One might expect the number of sweets in the bag to always equal the number put in minus the number taken out, i.e., the following should hold:

sweets = in - out, or
delta = in - out - sweets = 0

Currently the code prints out the value of delta every two seconds. It does not remain zero. Try it.
Assignment

There are two parts to the assignment...
1. Mutual exclusion

Incorporating Java's synchronized keyword, rewrite the code such that mutual exclusion is enforced where necessary. Once mutual exclusion is implemented properly you should see that delta remains permanently zero.
2. Avoiding busy-waiting

Incorporating Java's wait, notify and/or notifyAll methods to avoid busy-waiting, modify the code such that it takes three input parameters: min, max and start where:

* sweets is initialised to start, where min <= start <= max
* sweets must always be at least min (i.e. the Child should not take sweets from the bag when sweets == min)
* sweets must never exceed max (i.e. the Parent should not add sweets to the bag when sweets == max)

Sample run

When your program is working properly it might exhibit something like the following behaviour:

dobrien@vinson:> javac Assignment01.java
dobrien@vinson:> java Sweets
Usage: java Sweets min max start (where min <= start <= max)
dobrien@vinson:> java Sweets 1000 5000 100
Usage: java Sweets min max start (where min <= start <= max)
dobrien@vinson:> java Sweets 1000 5000 6000
Usage: java Sweets min max start (where min <= start <= max)
dobrien@vinson:> java Sweets 1000 5000 2000
Delta = 0 Sweets = 3968
Delta = 0 Sweets = 3643
Delta = 0 Sweets = 1871
Delta = 0 Sweets = 3065
Delta = 0 Sweets = 2779
Delta = 0 Sweets = 4955
Delta = 0 Sweets = 1022
Delta = 0 Sweets = 1065
Delta = 0 Sweets = 4959
Delta = 0 Sweets = 4965
Delta = 0 Sweets = 4823
Delta = 0 Sweets = 1002
Delta = 0 Sweets = 3272
Delta = 0 Sweets = 4953
Delta = 0 Sweets = 2801
Delta = 0 Sweets = 4643
Delta = 0 Sweets = 2554
etc.
Deliverables

You are required to deliver the following in a text file:

* Your Java source code solution in Assignment01.java

Marks are awarded for solving the assignment (it will be tested), solving it efficiently and demonstrating your understanding of the problems, issues and solutions through comments. It is particularly important that code which handles concurrency-related issues is well-commented. Any shortcomings in your solution must be clearly identified. Finally, your code must be neatly formatted, contain no lines longer than 80 characters and contain no commented-out code.

THE CODE

/**
* Title: Bag.java
* <p> Description: Defines a bag object, which may have sweets added and 
* removed from it, supplies all the relevant methods for adding, removing, 
* setting and getting of values. </p>
* @author Michael Dever
*/
import java.lang.IllegalArgumentException;
public class Bag
{
    private int sweets; // Current amount of sweets in the bag.
    private int in; // The amount of sweets that have been put into the bag.
    private int out; // The amount of sweets that have been taken from the bag.
    private int min; // The minimum amount of sweets than the bag must hold.
    private int max; // The maximum amount of sweets that the bag can hold.
    private int delta; // The delta value of this bag.

    public Bag( ) 
	{
		sweets = in = out = min = max = delta = 0; // All initialized to zero.
    }
    
    /**
	* Argumented constructor for Bag.
	* Requires minimum no. of sweets in the bag, max no. of sweets in the 
	* bag and the starting number of sweets in the bag. If the equation: 
	* <p><p><b>( min <= start <= max )</b></p></p>
	* is not satisfied, it will alert the user and terminate the program.
	* @param _min The minimum amount of sweets ever to be in bag.
	* @param _max The maximum amount of sweets ever to be in bag.
	* @param start The starting amount of sweets in the bag.
	*/
    
    public Bag( int _min, int _max, int start ) throws IllegalArgumentException
    {
    	if ( _min <= start && start <= _max )
    	{
    		min = _min; // Minimum amount of sweets in the bag.
    		max = _max; // Maximum amount of sweets in the bag.
    		sweets = in = start; // Starting amount of sweets in the bag.
    		out = delta = 0; // None taken yet.
    	}
		else
			throw new IllegalArgumentException( );
    }
    
    /**
	* Method to update the delta value of the bag, using the formula:
	* <p><p>( delta = in - out - sweets )</p></p>
	* This value should always be equal to zero.
	*/
    
    public synchronized void updateDelta( )
    {
    	delta = in - out - sweets; // Set delta according to the above formula.
    }
    
    /**
	* Method to put a sweet into the bag. If the bag is full it will 
	* wait until it is awoken when the bag is not full. When finished it 
	* notifies all threads waiting on this instance of bag, that bag has
	* updated.
	* @throws InterruptedException If it is interrupted while running.
	*/
    
    public synchronized void put( ) throws InterruptedException
    {
    	while ( sweets == max ) // While bag is full.
    		wait( ); // Wait.
   		this.sweets++; // Increase amount of sweets.
   		this.in++; // Increase amount put into bag.
   		updateDelta( ); // Update the delta value.
    	notifyAll( ); // Notify waiting threads.
    }
    
    /**
	* Method to take a sweet from the bag. If the bag contains only
	* the minimum amount of sweets that has to be in the bag, it will
	* wait until it is woken when there are sweets available for taking.
	* Once it has taken a sweet it notifies all other threads waiting on
	* this instance of bag that bag has been updated.
	* @throws InterruptedException If it is interrupted while executing.
	*/
    
    public synchronized void get( ) throws InterruptedException
    {
    	while ( sweets == min ) // While no more can be taken.
    		wait( ); // Wait.
    	// If at least one more can be removed.
   		this.sweets--; // Redudce amount of sweets.
   		this.out++; // Increase amount removed from bag.
   		updateDelta( ); // Update the delta value.
    	notifyAll( ); // Notify waiting threads.
    }
	
	/**
	* Prints info about the bags contents in the format:<br>
	* Delta = delta  Sweets = sweets<br>
	* This should be used rather than the get methods, as the thread may
	* have executed some more instructions between the particular calls of the
	* get methods, thereby printing untrue values.
	*/
	
	public synchronized void printInfo( )
	{
		System.out.println( "Delta: " + delta + " Sweets: " + sweets );
	}
    
    /**
	* Sets the amount of sweets that have gone into the bag.
	* @param _in The number of sweets that have gone into the bag.
	*/
    
    public synchronized void setIn( int _in )
    {
    	in = _in; // New amount of sweets put into the bag.
    }
    
    /**
	* Sets the amount of sweets in the bag.
	* @param _sweets The number of sweets in the bag. 
	*/
    
    public synchronized void setSweets( int _sweets )
    {
    	sweets = _sweets; // New amount of sweets in the bag.
    }
    
    /**
	* Sets the number of sweets that have left the bag.
	* @param _out The number of sweets that have left the bag.
	*/
    
    public synchronized void setOut( int _out )
    {
    	out = _out; // New amount of sweets gone out.
    }
    
    /**
	* Sets the minumum amount of sweets that can be in the bag.
	* @param _min The minimum number of sweets to be in the bag.
	*/
    
    public synchronized void setMin( int _min )
    {
    	min = _min; // New minimum amount of sweets that can be in the bag.
    }
    
    /**
	* Sets the maximum number of sweets that can be in the bag.
	* @param _max The maximum number of sweets that can be in the bag.
	*/	
    
    public synchronized void setMax( int _max )
    {
    	max = _max; // New maximum anmount of sweets that can be in the bag.
    }
    
    /**
	* Sets the delta value of the bag.
	* <p><p>( delta = sweets - in - out = 0 )</p></p>
	* This should always be zero.
	* @param _delta The new delta value of the bag.
	*/
    
    public synchronized void setDelta( int _delta )
    {
    	delta = _delta; // New delta value for the bag.
    }
    
    /**
	* Gets the current number of sweets that have gone into the bag.
	* @return The number of sweets that have entered the bag.
	*/
    
    public synchronized int getIn( )
    {
    	return in; // Return amount of sweets that have been put into the bag.
    }
    
    /**
	* Gets the current number of sweets that have left the bag.
	* @return The number of sweets that have left the bag.
	*/
    
    public synchronized int getOut( )
    {
    	return out; // Return amount of sweets that have been taken out.
    }
    
    /**
	* Gets the current number of sweets in the bag.
	* @return The number of sweets in the bag.
	*/
    
    public synchronized int getSweets( )
    {
    	return sweets; // Return amount of sweets in the bag.
    }
	
	/**
	* Gets the current minimum number of sweets that can be in the bag.
	* @return The minimum number of sweets that can be in the bag.
	*/
    
    public synchronized int getMin( )
    {
    	return min; // Return minimum amount of sweets that can be in the bag.
    }
	
	/**
	* Gets the current  maximum number of sweets that can be in the bag.
	* @return The maximum number of sweets that can be in the bag.
	*/
    
    public synchronized int getMax( )
    {
    	return max; // Return maximum amount of sweets that can be in the bag.
    }
	
	/**
	* Gets the current delta value of the bag.
	* @return The delta value of the bag.
	*/
    
    public synchronized int getDelta( )
    {
    	return delta; // Return the delta value of the bag.
    }
}
/**
* Title: Child.java
* <p>Description: Defines a child, that is constantly trying to remove sweets
* from a bag. The child runs in a thread, and has a lock on the bag it is
* trying to take sweets from, while it is trying to do so.</p>
* @author Michael Dever
*/

public class Child extends Thread
{
    private Bag bag; // Bag for this thread to work on.
	
	/**
	* Default no-args constructor for Child. Initialises its bag to null.
	*/
	
	public Child( )
	{
		bag = null;
	}

    /**
	* Argumented constructor for Child, requires  a bag for this Child to take
	* from.
	* @param _bag The bag to take sweets from.
	*/
    
    public Child( Bag _bag )
	{
		bag = _bag; // Assign the bag to be worked on.
    }

    /**
	* Tries to repeatedly take sweets from the bag. While the thread is alive
	* will try to take sweets from the bag, however if a problem arises, it
	* will print a message to console, alerting the user, and terminate.
	*/
    
    public void run( )
	{
    	while ( true ) // While it can...
    		try
    		{
    			bag.get( ); // Try to empty the bag.
    		}
    		catch( InterruptedException ie )
    		{ // If interrupted, alert user.
    			System.out.println( "Problem in Child.run( ) @ line 44."
									+ " Thread interrupted. Terminating." );
				System.exit( 0 ); // Terminate.
    		}
    }
	
	/**
	* Method for setting the bag this thread is working on.
	* @param _bag The bag that this thread is currently working on.
	*/
    
    public void setBag( Bag _bag )
    {
    	bag = _bag; // Set new bag.
    }
	
	/**
	* Method to get the bag this thread is working on.
	* @return The bag that this thread is currently working on.
	*/
    
    public Bag getBag( )
    {
    	return bag; // Return the bag that is cirrently being operated on.
    }
}
/**
* Title: Parent.java
* <p>Description: Defines a parent that is constantly trying to put sweets in
* a bag. The parent runs in a thread, and has a lock on the bag it is filling, 
* while it is filling it.</p>
* @author Michael Dever
*/

public class Parent extends Thread
{
    private Bag bag; // Bag for thread to work on.
	
	/**
	* Default no-args constructor for Parent. Initialises its bag to null.
	*/
	
	public Parent( )
	{
		bag = null; // Bag initialised to null.
	}

    /**
	* Argumented constructor for parent. Requires a bag for this parent to take
	* from.
	* @param _bag The bag to be filled with sweets.
	*/
    
    public Parent( Bag _bag )
	{
		bag = _bag; // Assign the bag to be operated on.
    }
    
    /**
	* Tries to fill the bag with sweets. While the thread is alive, it will try
	* to fill the bag with sweets. However if a problem arises it will print an
	* error message to the console alerting the user, and terminate.
	*/
    
    public void run( )
	{
    	while ( true ) // While it can...
    		try
    		{
    			bag.put( ); // Try to fill the bag.
    		}
    		catch( InterruptedException ie )
    		{ // If interrupted, alert user.
    			System.out.println( "Problem in Parent.run( ) @ line 44."
									+ " Thread interrupted. Terminating." );
				System.exit( 0 ); // Terminate
    		}
    }
	
	/**
	* Method for setting the bag this thread is working on.
	* @param _bag The bag for this thread to work on.
	*/
    
    public void setBag( Bag _bag )
    {
    	bag = _bag; // Set bag to new bag value.
    }
	
	/**
	* Method to get the bag this thread is working on.
	* @return The bag that this thread is currently working on.
	*/
    
    public Bag getBag( )
    {
    	return bag; // Return the bag that is currently being operated on.
    }
}
/**
* Title: Sweets.java
* <p>Description: Class to demonstrate the running of the Parent & Child <br>
* threaded classes. If first of all checks if all the requirements are <br>
* present. If not it alerts the user and then terminates.
* @author Michael Dever
*/

class Sweets 
{
    public static void main( String[] args ) throws InterruptedException 
	{
    	if ( args.length != 3 ) // Check if requirements are present.
    		System.out.println( "Usage: java Sweets min max start" );
    	else
    	{
			try
			{
		    	Bag bag = new Bag( Integer.parseInt( args[0] ), 
		    					   Integer.parseInt( args[1] ),
		    					   Integer.parseInt( args[2] ) ); // Get parameters
				Child cThread = new Child( bag ); // Child thread.
				Parent pThread = new Parent( bag ); // Parent thread.
				cThread.start(); // Start child thread.
				pThread.start(); // Start parent thread.
				while ( true ) 
				{
					Thread.sleep( 50 ); // Sleep for 1/2 second.
					bag.printInfo( ); // Print info about delta and sweets.
				}
			}
			catch ( IllegalArgumentException iae )
			{ // Print an alert.
				System.out.println( "Usage: java Sweets min max start"
									+ "\n\t where min <= start <= max." );
				System.exit( 0 ); // Terminate
			}
	    }
	}
}

We're not allowed use semaphores for this so don't suggest them lol, Im fairly sure I have all the bases covered on them, but there's no harm in asking if anyone else can suggest a way to improve.

Mick

In your Bag class, you use the synchronized keyword on every single method, but the get methods don't need it. Also, a set method doesn't need synchronized ONLY IF you are setting one of the variables that don't get changed in the updateDelta(), put(), or get() methods. So setMin(int _min) and setMax(int _max) don't need to have the synchronized keyword, but it can be a good idea to keep it there just in case your code changes so that the min and max variables can change, but you forget to replace the synchronized word.

So basiclly removed synchronized from all the get methods.

This article has been dead for over six months. Start a new discussion instead.