Timer with threads in java

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

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

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

You'll get this:

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 resolve this:

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

• In addition, I see that by resetting the variables startValue , stopValue and difTime to difTempo(String) , you are trying to clear the timer so that it can be reused in the future. When using instances this is no longer necessary, you simply throw away the old instance (the garbage collector will destroy it) and use a new one.

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

• By simply removing static from methods and attributes and adding an empty constructor (or just with the parameter name), the class would have to be used as constructor, start() , stop() , and difTime() , necessarily in that order. The first step in decreasing the temporal coupling would be to unify the constructor with start() .

• To further improve cohesion, I replace stop() with measureTimeElapsed() , which measures the time passed since the object was instantiated and returns it in the form of another object containing the method getDifTime() . With all of this I move on to a better level of cohesion (functional rather than temporal cohesion).

And then your class Temporizador looks 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 in upper case. Java naming conventions dictate that classes must have names beginning with uppercase letters, and it is often not a good idea to disobey the convention, even though it is allowed by the language.

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

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

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

Or if you want to start with the timer on:

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

However, since each client has its unique name and you will need one timer per client, then you will use Map , like this:

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 ConcurrentHashMap (of package java.util.concurrent ).

Below, your loop 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 define that values 1 and 1 in stopValue and difTime have the special meaning of saying start() was not called. Instead, if temp is null it is because the time has not yet begun to be counted, but if it is not null it is because it has already started, which is more intuitive.

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