Android buttons/app not working

Question

I must be doing something very stupid. This is my second app - the first worked fine (honest).

I've just got a basic title screen with a button on it but nothing happens when I click on the button. The screen appears correctly but no response from the button - I've tried it in debug but nothing...no error, no info message, zilch (I'm using the AVD).

This is my code...

Manifest:

XML file:

Java file:

public class TitleScreen extends Activity {

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

public void newGame (View v) {
    // create intent to start another activity
    Intent newGameIntent = new Intent(this, createNewGameActivity.class);

    startActivity(newGameIntent);
}
}

The only major difference between this and my other app is that I have now tried to use a single style file and I've re-designed my own buttons:

Anyone see the obvious mistake I must have made?

Xaver Kapeller · Accepted Answer

Personally I am really not a big fan of that android:onClick="newGame" stuff. You should try it the good old-fashioned way, like this:

public class TitleScreen extends Activity {

    private Button newBtn;

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

        this.newBtn = (Button) findViewById(R.id.newBtn);
        this.newBtn.setOnClickListener(new View.OnClickListener() {
            @Override
            public void onClick(View v) {
                Intent newGameIntent = new Intent(this, createNewGameActivity.class);
                startActivity(newGameIntent);   
            }
        });
    }
}

Or like this:

public class TitleScreen extends Activity implements View.OnClickListener {

    private Button newBtn;

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

        this.newBtn = (Button) findViewById(R.id.newBtn);
        this.newBtn.setOnClickListener(this);
    }

    @Override
    public void onClick(View v) {
        Intent newGameIntent = new Intent(this, createNewGameActivity.class);
        startActivity(newGameIntent);
    }
}

If you are dealing with multiple Buttons you can handle them in one OnClickListener like this:

private final View.OnClickListener clickListener = new View.OnClickListener() {
    @Override
    public void onClick(View view) {
        final int id = view.getId();
        switch (id) {

            case R.id.newBtn:
                Intent newGameIntent = new Intent(this, createNewGameActivity.class);
                startActivity(newGameIntent);
                break;

            case R.id.otherButton:
                Toast.makeText(this, "Other Button clicked!", Toast.LENGTH_SHORT).show();
                break;

            case R.id.cancelButton:
                ...
                break;
        }
    }
};

You get the id of the calling View and use a switch to differentiate between the specific Buttons. For example in your Activity you can implement this like this:

public class TitleScreen extends Activity implements View.OnClickListener {

    private Button newBtn;
    private Button otherBtn;
    private Button cancelBtn;

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

        this.newBtn = (Button) findViewById(R.id.newBtn);
        this.newBtn.setOnClickListener(this);

        this.otherBtn = (Button) findViewById(R.id.otherBtn);
        this.otherBtn.setOnClickListener(this);

        this.cancelBtn = (Button) findViewById(R.id.cancelBtn);
        this.cancelBtn.setOnClickListener(this);
    }

    @Override
    public void onClick(View v) {
        final int id = view.getId();
        switch (id) {

            case R.id.newBtn:
                Intent newGameIntent = new Intent(this, createNewGameActivity.class);
                startActivity(newGameIntent);
                break;

            case R.id.otherBtn:
                Toast.makeText(this, "Other Button clicked!", Toast.LENGTH_SHORT).show();
                break;

            case R.id.cancelBtn:
                ...
                break;
        }        
    }
}

A few things that caught my eye:

The names of classes are supposed to start with an upper case letter, so instead of createNewGameActivity you Activity should be called CreateNewGameActivity.
Use Fragments. The golden age of Activities is over. Since Fragments were introduced Activities are just supposed to be containers for the Fragments. So even if you just have one Button in your layout you should have an empty Activity which just contains one Fragment and in that Fragment you implement your UI and logic.
Don't use android:onClick. There is no indication in the code that this method will be called at all. And there is no reason for the layout to assume that such a method exists anywhere in the View, Fragment or Activity which loads the layout. And aside from that it isn't even that convenient. In 99,9% of all cases in which you need to set an OnClickListener you also need to do other stuff with the Button and then you need the reference anyway. android:onClick just clutters your code and makes it more confusing. it spreads implementation details out and obscures it to the developer. That the layout knows or even defines specific implementation details is in itself already pretty bad.

Android buttons/app not working

Answers (1)

Related Questions