5
\$\begingroup\$

The app: Creates CountDownTimers and displays them in a simple layout. The user can choose how long the timer should be through SharedPreferences and a settings activity. Each timer has a preparation period before it actually starts.

The problem: I'm not sure how, but I feel like the code is messy and needs to be cleaned up. In particular, the CountDownTimer is nested five layers deep:

public void startSession(View v) {
    if (!isTicking) {
        // Runs on play press
        countDownTimer = new CountDownTimer(session.getPrepRemaining() + session.getMindRemaining(), 10) {

            @Override
            public void onTick(long millisUntilFinished) {
                if (millisUntilFinished > session.getMindRemaining()) {
                    // Preparation timer
                    session.setPrepRemaining(millisUntilFinished - session.getMindRemaining());
                    setPrepDigits(millisUntilFinished - session.getMindRemaining());
                } else {
                    // Mindfulness timer
                    prepDigits.setVisibility(View.INVISIBLE);
                    session.setMindRemaining(millisUntilFinished);
                    circularProgressBar.setProgress(((session.getMindLength() - millisUntilFinished) / (float) session.getMindLength()) * 100);
                    setMindDigits(millisUntilFinished);
                }
            }

            @Override
            public void onFinish() {
                session = null;
                isTicking = false;
                circularProgressBar.setProgress(100);
                initializeSession();
            }
        };
        countDownTimer.start();
        isTicking = true;
        startPauseButton.setImageDrawable(getResources().getDrawable(android.R.drawable.ic_media_pause));

        // Alarm
        AlarmReceiver.scheduleAlarm(this);
    } else {
        // Runs on pause press
        countDownTimer.cancel();
        isTicking = false;
        startPauseButton.setImageDrawable(getResources().getDrawable(android.R.drawable.ic_media_play));
    }
}

If you'd like to see the rest of the code, please check out the gist.

It seems like I should have one class managing the counting and another class managing UI, but again, I'm just not sure what steps I should take to make that happen. I also feel like there are several blocks of code that could be pulled out into their own methods, but I feel like they'd be too entwined with, for example, the timer itself.

I've been doing a lot of reading on clean code, and I'm aware of the principles, but I don't know how to put them into practice. So, how can I organize, simplify, and clarify my code?

\$\endgroup\$
2
\$\begingroup\$

I think you need to separate the CountDownTimer class logic instead of anonymous implementation. I just made the below sample code which I try to loose coupled the CountDownTimer from the Activity

import android.annotation.SuppressLint;
import android.annotation.TargetApi;
import android.app.Activity;
import android.os.Build;
import android.os.Bundle;
import android.os.CountDownTimer;
import android.view.View;
import android.view.View.OnClickListener;
import android.widget.Button;
import android.widget.TextView;

import java.util.concurrent.TimeUnit;

@TargetApi(Build.VERSION_CODES.GINGERBREAD)
@SuppressLint("NewApi")
public class MainActivity extends Activity {
    Button btnStart, btnStop;
    TextView textViewTime;

    @Override
    public void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);
        btnStart = (Button) findViewById(R.id.btnStart);
        btnStop = (Button) findViewById(R.id.btnStop);
        textViewTime = (TextView) findViewById(R.id.textViewTime);
        textViewTime.setText("00:03:00");
        final CounterClass timer = new CounterClass(180000, 1000);
        btnStart.setOnClickListener(new OnClickListener() {
            @Override
            public void onClick(View v) {
                timer.start();
            }
        });
        btnStop.setOnClickListener(new OnClickListener() {
            @Override
            public void onClick(View v) {
                timer.cancel();
            }
        });
    }

    @TargetApi(Build.VERSION_CODES.GINGERBREAD)
    @SuppressLint("NewApi")
    public class CounterClass extends CountDownTimer {
        public CounterClass(long millisInFuture, long countDownInterval) {
            super(millisInFuture, countDownInterval);
        }

        @Override
        public void onFinish() {
            textViewTime.setText("Completed.");
        }

        @SuppressLint("NewApi")
        @TargetApi(Build.VERSION_CODES.GINGERBREAD)
        @Override
        public void onTick(long millisUntilFinished) {
            long millis = millisUntilFinished;
            String hms = String.format("%02d:%02d:%02d", TimeUnit.MILLISECONDS.toHours(millis),
                    TimeUnit.MILLISECONDS.toMinutes(millis) - TimeUnit.HOURS.toMinutes(TimeUnit.MILLISECONDS.toHours(millis)),
                    TimeUnit.MILLISECONDS.toSeconds(millis) - TimeUnit.MINUTES.toSeconds(TimeUnit.MILLISECONDS.toMinutes(millis)));
            System.out.println(hms);
            textViewTime.setText(hms);
        }
    }
}
| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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