2
\$\begingroup\$

This project was made to replace the stock Android CountdownTimer class. It is more responsive when completing the timer and adds additional functionality such as lap counting and start, pause, resume, and stop methods.

However, I am not so familiar with threads and was wondering if I am doing it right and how it can be improved.

I will post the MainActivity where the timer is being used as well as my CustomTimer class.

CustomTimer.java

package CustomTimer;

import android.app.Activity;

/**
 * Created by moshe on 5/12/2016.
 */
public class CustomTimer extends Thread {

    public interface TimerListener{
        public void newLap(int lapNumber);
        public void onTick(long timeRemainingMillis);
    }

    private TimerListener listener;
    private Activity activity;

    private long timerStartMillis;
    private long timeRemainingMillis;
    private long lapLengthMillis;
    private int  lapNumber;
    private long previousTickMillis;
    private long tickLength;
    private long timePaused;

    private boolean clockRunning;
    private boolean clockBegan;

    public CustomTimer(Activity activity) {
        super();
        this.activity = activity;
        lapLengthMillis = 0;
        timerStartMillis = 0;
        lapNumber = 0;
        timeRemainingMillis = 0;
        previousTickMillis = 0;
    }

    public void setListener(TimerListener listener){
        this.listener = listener;
    }

    public void setLapLengthMillis(long lapLengthMillis){
        this.lapLengthMillis = lapLengthMillis;
    }

    @Override
    public void run() {
        super.run();
        while(true) {
            while (clockBegan) {
                if (clockRunning) {
                    long currentTime = System.currentTimeMillis();
                    timeRemainingMillis = (timerStartMillis + lapLengthMillis * (lapNumber + 1)) - currentTime;

                    if (currentTime - previousTickMillis >= tickLength) {
                        previousTickMillis = currentTime;
                        activity.runOnUiThread(new Runnable() {
                            @Override
                            public void run() {
                                listener.onTick(timeRemainingMillis);
                            }
                        });
                        //Log.d("Tick happening", "" + timeRemainingMillis);
                    }
                    if (timeRemainingMillis < 0) {
                        lapNumber++;
                        timeRemainingMillis = System.currentTimeMillis();
                        activity.runOnUiThread(new Runnable() {

                            @Override
                            public void run() {
                                listener.newLap(lapNumber);
                            }
                        });
                        //Log.d("New lap happening", ""+lapNumber);
                    }
                }
            }
        }
    }

    public void setTickLength(long tickLength) {
        this.tickLength = tickLength;
    }

    public boolean isClockRunning(){
        return clockRunning;
    }

    public void stopClock() {
        clockBegan = false;
        clockRunning = false;
    }
    public void pauseClock(){
        clockRunning = false;
        timePaused = System.currentTimeMillis();
    }

    public void startClock(){
        if(!clockBegan) {
            timerStartMillis = System.currentTimeMillis();
            previousTickMillis = timerStartMillis;
            lapNumber = 0;
            clockRunning = true;
            clockBegan = true;
            if(State.NEW == this.getState())
                super.start();
        }else{
            resumeClock();
        }
    }

    public  void resumeClock(){
        clockRunning = true;
        timerStartMillis += (System.currentTimeMillis() - timePaused);
    }
}

MainActivity.java

package com.itnstudios.customtimer;

import android.os.Bundle;
import android.support.design.widget.FloatingActionButton;
import android.support.design.widget.Snackbar;
import android.support.v7.app.AppCompatActivity;
import android.support.v7.widget.Toolbar;
import android.util.Log;
import android.view.View;
import android.view.Menu;
import android.view.MenuItem;
import android.widget.Button;
import android.widget.TextView;
import android.widget.Toast;

import CustomTimer.CustomTimer;

public class MainActivity extends AppCompatActivity {

    CustomTimer timer;
    TextView tvClock;
    Button startButton, pauseButton, resumeButton;

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);

        startButton = (Button) findViewById(R.id.btn_start);
        pauseButton = (Button) findViewById(R.id.btn_pause);
        resumeButton = (Button) findViewById(R.id.btn_resume);

        tvClock = (TextView) findViewById(R.id.tv_clock);
        updateClock(5000);

        timer = new CustomTimer(this);
        timer.setLapLengthMillis(5000);
        timer.setTickLength(500);
        timer.setListener(new CustomTimer.TimerListener() {
            @Override
            public void newLap(int lapNumber) {
                Toast.makeText(getApplicationContext(), "Lap Number: " + lapNumber, Toast.LENGTH_SHORT).show();
            }

            @Override
            public void onTick(long timeRemainingMillis) {
                updateClock(timeRemainingMillis);
                Log.d("Tick Happening", timeRemainingMillis+"");
            }
        });

    }

    @Override
    public boolean onCreateOptionsMenu(Menu menu) {
        // Inflate the menu; this adds items to the action bar if it is present.
        getMenuInflater().inflate(R.menu.menu_main, menu);
        return true;
    }

    @Override
    public boolean onOptionsItemSelected(MenuItem item) {
        // Handle action bar item clicks here. The action bar will
        // automatically handle clicks on the Home/Up button, so long
        // as you specify a parent activity in AndroidManifest.xml.
        int id = item.getItemId();

        //noinspection SimplifiableIfStatement
        if (id == R.id.action_settings) {
            return true;
        }

        return super.onOptionsItemSelected(item);
    }

    private void updateClock(long timeInMillis) {
        int min = (int) (timeInMillis / 60000);
        int sec = (int) ((timeInMillis % 60000) / 1000);
        tvClock.setText(String.format("%02d:%02d", min, sec));
    }

    public void startClock(View v){
        timer.startClock();
        v.setVisibility(View.GONE);
        pauseButton.setVisibility(View.VISIBLE);
    }

    public void pauseClock(View v){
        timer.pauseClock();
        v.setVisibility(View.GONE);
        resumeButton.setVisibility(View.VISIBLE);
    }

    public void resumeClock(View v){
        timer.resumeClock();
        v.setVisibility(View.GONE);
        pauseButton.setVisibility(View.VISIBLE);
    }

    public void stopClock(View v){
        timer.stopClock();
        resumeButton.setVisibility(View.GONE);
        pauseButton.setVisibility(View.GONE);
        startButton.setVisibility(View.VISIBLE);
    }
}
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

So you're not exactly thread safe. Even though you're extending Thread, that does not mean that its own methods will be called within it's ownership. For example, if you had a super simple class like this:

public class TestThread extends Thread {
    public void test(){
        System.out.println(Thread.currentThread().getId() + ":" + getId());
    }
}

and instantiate and call it from somewhere else:

new TestThread().test();

you'll see that it's probably not going to match.

Thread "safety" can be very complicated. One major consideration is "memory consistency", where each thread might make a cache copy of information you have in memory, and so might write to them at different times. Often this isn't even noticeable, but if the order or state of a variable matters, it can get tricky. This might apply to you since you're updating a lot of variables, inherently between processes.

I'd spend some time reading up on thread safety in Java (there are a ton of resources online, all of them will do a better job explaining than I have), and I probably wouldn't extend Thread here - I think most people would probably use a Thread but the class itself would not be a subclass.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.