3

I have this assignment which is to simulate the behaviour of a printer where print jobs may be submitted to a printer with varying time intervals or simultaneously. So, thread.start() must be used here, and the printer will take a job from the printer queue and print it, while the rest of the other jobs wait for their turn.

In total I only need to write 3 classes, the Admin class, the PrintRequest class and the Printer class.

Here is my codes:

public class Admin{
    public static void main(String[] args){
        Printer printer = new Printer();
        for(int x=0; x<10;x++){
            PrintRequest printRequest = new PrintRequest(x,printer);
            printRequest.start();
        }
    }
}

public class PrintRequest extends Thread {
    private static int duration; 
    private static int id;
    private static Printer printer;

    public PrintRequest(int id, Printer printer){
        duration = (int)Math.ceil(Math.random()*1000);
        this.id = id;
        this.printer = printer;
    }
    public void run(){
        printer.printJob(id, duration);
    }
}

public class Printer{
    public static synchronized void printJob(int id, int duration){
        System.out.println("Printing " + id);
        try{Thread.sleep(duration);}
        catch(InterruptedException ex){System.out.println(ex.getMessage());}
        System.out.println("Completed printing " + id);
    }
}

There is some problems with the output, it is not the desired output that I want -- printing from ID 0-10. Instead the output is as such:

enter image description here

So what is the problem in my code? I would think it is the problem with the Admin class loop but how should I improve on it such that it can function correctly?

Stack Danny
  • 7,754
  • 2
  • 26
  • 55
RandomDude
  • 31
  • 2
  • printer executes jobs sequentially, so it is a bad idea to represent job as a thread. Threads are inherently parallel, and here you have to spend efforts to suppress that parallelism. Better way is to start single thread which represent the printer, and make a queue for print jobs. – Alexei Kaigorodov Apr 16 '19 at 10:22
  • So you are suggesting that I extend the printer class as the thread right? The second part on the queue, are there any examples of its implementation? Thanks in advance! – RandomDude Apr 16 '19 at 10:25
  • This seems to be a variation of the Producer-Consumer problem. – Soutzikevich Apr 16 '19 at 10:56
  • @RandomDude see, for example, https://kodejava.org/how-do-i-use-the-blockingqueue-object/ – Alexei Kaigorodov Apr 16 '19 at 12:53
  • Re, "...extend the printer class as the thread..." That would work, but a better design (especially if you were trying to build real, commercial-quality software), would be to separate the idea of a `Printer` from the thread that runs it. I would define a `Printer` class with a constructor that takes a `PrintQueue` argument, and a `run()` method. my `Printer` would know nothing about threads. Then I would make a higher-level "glue" routine that would create a `PrintQueue` instance, create a `Printer` to serve it, create a `Thread` to run the `Printer`, and connect them all together. – Solomon Slow Apr 16 '19 at 13:03
  • P.S., My biggest reason for defining it that way is testing. I usually have to test the software that I write. It would be *much* easier to test a `Printer` that knows nothing about threads than, to test a `Printer` that _is_ a thread. – Solomon Slow Apr 16 '19 at 13:10
  • The second part of the question was to try this with ReentrantLock, where the reentrant lock seems to have created some form of queue for the printer function. Thanks guys for your advice! – RandomDude Apr 17 '19 at 03:04

1 Answers1

2

There are several problems with your code.

First, I think you should lookup what a static variable is. As it is, all your PrintRequests have the same id, I don't think that's what you want.

Also, your line that generates the sleep duration only generates number between 0 and 1000. This means that a job lasts less than a second, and your output may not look like what you expect.

Change your line to 

duration = (int)(1000 + (Math.random()*5000));

This gave me the following output

Printing 0
Printing 2
Printing 1
Printing 4
Printing 5
Printing 6
Printing 7
Printing 8
Printing 3
Printing 9
Completed printing 6
Completed printing 4
Completed printing 8
Completed printing 2
Completed printing 9
Completed printing 3
Completed printing 1
Completed printing 0
Completed printing 7
Completed printing 5

You may ask : why isn't printing in order ?

That's because you cannot control sequentially the order in which Java will launch your threads in a loop like you've made. If you really want to, you will have to add some more mechanisms to enhance that.

Arthur Attout
  • 2,701
  • 2
  • 26
  • 49