2
\$\begingroup\$

Since I am trying to learn many programming languages, I created this simple Android app. What it does basically is taking a sentence phrase from an EditText ed and translate it to speech when you click the start button.

I used onActivityResult to prevent app from crashing when the EditText becomes empty.

I am sensing that this code, while working perfectly, is far from perfect.

import android.support.v7.app.AppCompatActivity;
import java.util.Locale;
import android.content.Intent;
import android.os.Bundle;
import android.speech.tts.TextToSpeech;
import android.speech.tts.TextToSpeech.OnInitListener;
import android.view.View;
import android.widget.Button;
import android.widget.EditText;
import android.widget.Toast;

public class MainActivity extends AppCompatActivity implements OnInitListener {

private TextToSpeech myTTS;
private EditText ed;
private String phrase;

@Override
protected void onCreate(Bundle savedInstanceState) {

    super.onCreate(savedInstanceState);
    setContentView(R.layout.activity_main);


    ed = findViewById(R.id.editText);

    Button startButton = findViewById(R.id.start_button);
    startButton.setOnClickListener(new View.OnClickListener() {
        @Override
        public void onClick(View arg0) {

            Intent checkIntent = new Intent();
            checkIntent.setAction(TextToSpeech.Engine.ACTION_CHECK_TTS_DATA);
            startActivityForResult(checkIntent, 1);

            phrase = ed.getText().toString();

        }
    });
}


protected void onActivityResult(int requestCode, int resultCode, Intent data) {
    if (requestCode == 1) {

        if (resultCode == TextToSpeech.Engine.CHECK_VOICE_DATA_PASS) {
            myTTS = new TextToSpeech(this, this);
            myTTS.setLanguage(Locale.US);
        } else {
            // TTS data not yet loaded, try to install it
            Intent ttsLoadIntent = new Intent();
            ttsLoadIntent.setAction(TextToSpeech.Engine.ACTION_INSTALL_TTS_DATA);
            startActivity(ttsLoadIntent);
        }
    }
}

public void onInit(int status) {
    if (status == TextToSpeech.SUCCESS) {
        myTTS.speak(phrase, TextToSpeech.QUEUE_FLUSH, null);
    } else if (status == TextToSpeech.ERROR) {
        myTTS.shutdown();
    }
}

}

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

First of all better use clear variable names. Coming from android I can guess what an 'ed' is. But for everybody it is always better to name it properly like :

private EditText editText;
editText = findViewById(R.id.editText);

or in this case maybe even

editTextTTS;

or something similar. Same with myTTS, I personally would also name that :

private TextToSpeech textToSpeech;

You are creating a TextToSpeech Object on every click, which is absolutely unneccessary, create it once when starting the application, and reuse it. So this piece of code belongs in your onCreate() not your onClick() :

Intent checkIntent = new Intent();
checkIntent.setAction(TextToSpeech.Engine.ACTION_CHECK_TTS_DATA);
startActivityForResult(checkIntent, 1);

That way the TTS will be ready before the user can click the button and is only initialised once.

You are getting the value of your editText AFTER you startActivityForResult(). This does not seem to be the logical order, although if you remove the code above from your onClick(), that also solves this problem.

You have a magic number 1 in your startActivityForResult() method, better use a final value with a proper name like :

private final int TTS_REQUEST_CODE = 1;

makes this also look way better :

startActivityForResult(checkIntent, TTS_REQUEST_CODE);

if (requestCode == TTS_REQUEST_CODE) 

In case the application becomes larger and more complex this will be way more secure and readable since the value is final and cannot be tampered with.

There is no information to the user wether the TextToSpeech is installed and ready or not, I would suggest showing a Toast or a Snackbar to let the user know what is happening, especially in case of

status == TextToSpeech.ERROR

You might even want to smooth exit the application in this case.

This app will only please Users with US English language. Why not make it get the Locale from the Users device, to set the proper language dynamically :

myTTS.setLanguage(Locale.getDefault());

The onInit() method should only be called once when initialising the TTS Object, so your

myTTS.speak(phrase, TextToSpeech.QUEUE_FLUSH, null);

is misplaced there. This code should go into your onClick() method along with a check that the editText is not empty :

public void onClick(View arg0) {

    final String phrase = ed.getText().toString();
    if(!phrase.isEmpty()) {
        myTTS.speak(phrase, TextToSpeech.QUEUE_FLUSH, null);
    } else {
        // Toast or Snackbar to tell the user Edittext is empty
    }

This will also convert

private String phrase;

to a local final variable, which is always better than having an unneccessary global variable.

\$\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.