Java threaded timer

Question

Good night, you guys!

I am developing a multithread client/server system. Multiple clients connect and when they send the string "Ocupada" 3 times, the code leaves the while, starts the time count and ends the connection with it. When the customer is turned on again, if the shipment is not "Busy", it shows the time that was turned off. Code below:

 while (entrada.hasNextLine() && this.cont < 3) {

                saida.println("Situação?");
                String sit = entrada.nextLine();
                System.out.println(sit);//recebe situação

                if ("Ocupada".equals(sit)) {                    
                    this.cont++;                    
                } else if(temporizador.getStop() == 0 && temporizador.getDifTime() == 0 ) { // faz o stop e exibe tempo                                                            
                    temporizador.stop();
                    temporizador.difTempo(this.nomeDispositivo);                    
                }else{
                    this.cont = 0;
                }
                System.out.println(this.cont);                            
            }
            //inicia a contagem aqui e só exibe quando o temporizador.stop() for chamado ( dentro do while)
            if (temporizador.getStart() == 0){
            temporizador.start();
                System.out.println("Start no tempo!");
           }

The timer class is as follows:

public class temporizador {

    private static long startValue = 0;
    private static long stopValue = 1;
    private static long difTime = 1;

    public static void start() {
        startValue = System.currentTimeMillis();
        stopValue = 0;
        difTime = 0;
    }

    public static void stop() {
        stopValue = System.currentTimeMillis();
        difTime = stopValue - startValue;            
    }

    public static void difTempo(String nome) throws SQLException {
        String format = String.format("%02d:%02d:%02d", difTime / 3600000, (difTime / 60000) % 60, (difTime / 1000) % 60);
        System.out.println(nome + " levou " + format);

        startValue = 0;
        stopValue = 1;
        difTime = 1;
    }

    public static long getStart(){
        return startValue;
    }
     public static long getStop(){
        return stopValue;
    }
     public static long getDifTime(){
        return difTime;
    }
}

It is working perfectly but for one customer only, since the count does not go according when more than one customer sends "Ocupada".

I would like a help to implement the timer as a thread, so that the various customers access and display the time of each separately.

The program already counts separately the number of times each client sent the string "Ocupada" by means of the variable cont. However, on the timer this does not happen. For a customer the count is done perfectly, only that the values do not go according when more than one customer accesses.

Within the while, the block else if is a gambiarra I did (I still think it was the best option) that ensures that the temporizador.stop() and temporizador.difTime(String) do not execute before a temporizador.start(), since at the beginning of the timer is stopValue = 1 and difTime = 1 and the this.cont = 0 Zera the count in case the customer sends something other than "Ocupada".

        Temporizador temp = temporizadores.get(this.nomeDispositivo);            
        System.out.println(temp);
        while (entrada.hasNextLine() && this.cont < 3) {

            saida.println("Situação?");
            String sit = entrada.nextLine();
            System.out.println(sit); // Mostra a situação

            if ("Ocupada".equals(sit)) {
                this.cont++;
            } else if (temp != null) { // Para o temporizador e exibe o tempo.
                System.out.println(temp.measureTimeElapsed().getReport());
                temp = null;
                temporizadores.put(this.nomeDispositivo, null);
            } else {
                this.cont = 0;
            }
            System.out.println(this.cont);// Contagem das vezes que esteve ocupada

            String sql2 = "UPDATE vagas SET situacao = (?) WHERE nomeID = (?)";

            try (PreparedStatement stmt = conecta.conn.prepareStatement(sql2)) {
                stmt.setString(1, sit);
                stmt.setString(2, this.nomeDispositivo);
                stmt.executeUpdate();
            }
        }            
 //------------------------------------------------------------------------------            
// Inicia a contagem aqui e só exibe quando o measureTimeElapsed() for chamado (dentro do while).            
        if (temp == null) {
            temp = new Temporizador(this.nomeDispositivo);                
            temporizadores.put(this.nomeDispositivo, temp);
            System.out.println("Start no tempo!");
            System.out.println(temporizadores);
        }

Answer 1

I noticed your class temporizador contains all static methods and attributes, which means that the same timer is shared across all clients. However, you seem to want each customer to have their own timer.

Let’s look at the behavior of your timer. The method start() should be called first, then the stop() should be called and finally the difTime(String) obligatorily and necessarily in that order, otherwise the timer will go crazy. This is an example of temporal cohesion (see about this in that reply), which is bad. And that’s also why your program fails, because with multiple threads instead of you having this:

start(); stop(); difTime(String); start(); stop(); difTime(String); start(); stop(); difTime(String); ...

Will have it:

start(); start(); stop(); start(); difTime(String); stop(); stop(); start(); difTime(String); difTime(String); stop(); ...

And the result will be chaos.

Let’s see what we can do to fix this:

• The first idea would be to use instances of the timer instead of keeping everything static. Thus, each client can have its own timer.

• Also, I see that when resetting the variables startValue, stopValue and difTime in the method difTempo(String), You’re trying to clean the timer so it can be reused in the future. When using instances this is no longer necessary, just throw out the old instance (the garbage collector will destroy it) and use a new one.

• The name of the process being measured may be inside the timer itself instead of being informed only in the method difTempo. Thus, the name would become a parameter of the timer constructor.

• Simply by removing the static of the methods and attributes and add an empty constructor (or only with the name as parameter), the class would have to be used as constructor, start(), stop() and difTime(), necessarily in that order. The first step to decrease temporal coupling would then be to unify the constructor with the start().

• To further improve cohesion, I replace the stop() by a measureTimeElapsed(), which measures the time passed since the object was instantiated and returns it in the form of another object, which contains the method getDifTime(). With all this I have a better level of cohesion (functional cohesion instead of temporal).

And then your class Temporizador gets like this:

public final class Temporizador {

    private final String nome;
    private final long startValue;

    public Temporizador(String nome) {
        this.nome = nome;
        startValue = System.currentTimeMillis();
    }

    public MeasuredTime measureTimeElapsed() {
        return new MeasuredTime(this);          
    }

    public long getStart() {
        return startValue;
    }

    public String getNome() {
        return nome;
    }
}

public final class MeasuredTime {

    private final Temporizador temp;
    private final long endValue;

    MeasuredTime(Temporizador temp) {
        this.temp = temp;
        this.endValue = System.currentTimeMillis();
    }

    public long getStart() {
        return temp.getStart();
    }

    public long getEnd() {
        return endValue;
    }

    public long getDifTime() {
        return getEnd() - getStart();
    }

    public String getReport() {
        long difTime = getDifTime();
        long horas = difTime / 3_600_000;
        long minutos = (difTime / 60_000) % 60;
        long segundos = (difTime / 1_000) % 60;
        //long millis = difTime % 1_000;
        String format = String.format("%02d:%02d:%02d", horas, minutos, segundos);
        return temp.getNome() + " levou " + format;
    }
}

Note that I put Temporizador capital letters. Naming conventions in Java dictate that classes should have names starting with uppercase letters, and it is not usually a good idea to disobey the convention, although it is permitted by the language.

Also note that the classes Temporizador and MeasuredTime are immutable, and therefore are much easier to use and test and can even be shared among many threads (although in your case, this is not what you want).

To use your timer somewhere before the while you will need to declare it properly:

// Em algum lugar você coloca isso:
Temporizador temp = null;

Or if you already want to start with the timer on:

// Em algum lugar você coloca isso:
Temporizador temp = new Temporizador(this.nomeDispositivo);

However, since each customer has their own unique name and you will need one timer per customer, then you will use one Map, thus:

private final Map<String, Temporizador> temporizadores;

In the class constructor you initialize it:

temporizadores = new ConcurrentHashMap<>(16);

Note that the most commonly used implementations (HashMap or LinkedHashMap) are not conducive to being accessed by multiple threads, and because of this we use the ConcurrentHashMap (package java.util.concurrent).

Further down, your bow looks like this:

Temporizador temp = temporizadores.get(this.nomeDispositivo);

while (entrada.hasNextLine() && this.cont < 3) {
    saida.println("Situação?");
    String sit = entrada.nextLine();
    System.out.println(sit); // Mostra a situação

    if ("Ocupada".equals(sit)) {
        this.cont++;
    } else if (temp != null) { // Para o temporizador e exibe o tempo.
        System.out.println(temp.measureTimeElapsed().getReport());
        temp = null;
        temporizadores.put(this.nomeDispositivo, null);
    } else {
        this.cont = 0;
    }
    System.out.println(this.cont);
}

// Inicia a contagem aqui e só exibe quando o measureTimeElapsed() for chamado (dentro do while).
if (temp == null) {
    temp = new Temporizador(this.nomeDispositivo);
    temporizadores.put(this.nomeDispositivo, temp);
    System.out.println("Start no tempo!");
}

Note that with this you no longer need to set the values 1 and 1 in stopValue and in the difTime have the special meaning of saying that the start() was not called. Instead, if the temp for null it is because time has not yet begun to be counted, but if it is not null is because it’s already started, which is more intuitive.

There may be a lot of other possible improvements to this loop or other parts of your code that are correlated, but for that you would need more information about the context in which it is used, and it would probably already be the subject of another question.