CODEWITHSUNDEEP
javaandroid
Si8
Si8

Reputation: 9225

Make app more effecient by eliminating extra codes

I currently have the following code which starts from A all the way to Z :

if (someId.matches("A") || someId.matches("a")) {
    tvLetCap.setText("A");
    tvLetLow.setText("a");
    ivLetterIcon.setImageResource(R.drawable.apple);
    btnDisplayWord.setText("A is for APPLE");
    btnPlay.setOnClickListener(new View.OnClickListener() {
        public void onClick(View v) {
            stopPlaying();
            mpSound = MediaPlayer.create(MyClass.this, R.raw.a);
            mpSound.setLooping(false);
            mpSound.start();
            btnPlay.setVisibility(View.GONE);
            btnStop.setVisibility(View.VISIBLE);
            btnStop.setOnClickListener(stopSound);
            mpSound.setOnCompletionListener(new OnCompletionListener() {
                @Override
                public void onCompletion(MediaPlayer mp) {
                    btnPlay.setVisibility(View.VISIBLE);
                    btnStop.setVisibility(View.GONE);
                }
            });
        }
    });
}       
if (someId.matches("B") || someId.matches("b")) {
    tvLetCap.setText("B");
    tvLetLow.setText("b");
    ivLetterIcon.setImageResource(R.drawable.ball);
    btnDisplayWord.setText("B is for BALL");
    btnPlay.setOnClickListener(new View.OnClickListener() {
        public void onClick(View v) {
                stopPlaying();
                mpSound = MediaPlayer.create(MyClass.this, R.raw.b);
            mpSound.setLooping(false);
                mpSound.start();
            btnPlay.setVisibility(View.GONE);
            btnStop.setVisibility(View.VISIBLE);
            btnStop.setOnClickListener(stopSound);
            mpSound.setOnCompletionListener(new OnCompletionListener() {
                @Override
                public void onCompletion(MediaPlayer mp) {
                    btnPlay.setVisibility(View.VISIBLE);
                    btnStop.setVisibility(View.GONE);
                }
            });
        }
    });
}

Would this work as oppose to the above:

switch(someId) {
    case A:
        setLetter("A");
        addIcon("apple");
        break;
    case B:
        setLetter("B");
        addIcon("ball");
        break;
    default:
        break;
}

public void setLetter(String strLetter) {
    tvLetCap.setText(strLetter);
    tvLetLow.setText(strLetter.toLowerCase());
}
public void addIcon(String iconLetter) {
    ivLetterIcon.setImageResource(R.drawable. + iconLetter);
    btnDisplayWord.setText(iconLetter.charAt(0).toUpperCase() + " is for " + iconLetter.toUpperCase());
}

I am guessing the only issue might be with this line and how would I fix it?:

ivLetterIcon.setImageResource(R.drawable. + iconLetter);

Also will it be possible to take the entire btnPlay function to a different function and just pass the letter like I did with the other functions so it's not repeated over?

Upvotes: 0

Views: 85

Answers (2)

ajb
ajb

Reputation: 31699

I don't know what R and R.drawable are; and what are R.drawable.apple, R.drawable.ball, etc.? Assuming those all have the same type, you might want to make R.drawable an array where the index is 0 for 'A', 1 for 'B', etc., and initialize it so that R.drawable[0] = (whatever the "apple" is supposed to be), R.drawable[1] = (same for "ball"); then use something like 

R.drawable[someId.charAt(0).toUpperCase() - 'A']

and similarly for R.raw. Then you wouldn't need a switch at all.

EDIT by kcoppock:

For example:

int[] icons = {
    R.drawable.a,
    R.drawable.b,
    //etc.
};

int[] sounds = {
    R.raw.a,
    R.raw.b,
    //etc.
}

and use icons[index] and sounds[index] to map your values.

Upvotes: 4

Coeffect
Coeffect

Reputation: 8866

Simplest solution: add another parameter to addIcon:

public void addIcon(String iconLetter, int iconResource){
    ivLetterIcon.setImageResource(iconResource);
    btnDisplayWord.setText(iconLetter.charAt(0).toUpperCase() + " is for " + iconLetter.toUpperCase());
}

Then call it with

setIcon("apple", R.drawable.apple);

As far as reusing the on click listener, it's certainly possible. You could have a single onClick listener that's listening to all the views. When a view is clicked, it's passed in as the argument to the onClick(View view) method. You can use that to find the start/stop button for that view and do whatever to them. Also, there's a useful (and often abused) method pair: setTag(Object object), and getTag(). You might want to look into using those to store the resource ID for the raw audio file you want to play when a button is clicked.

Upvotes: 1

Related Questions