4
\$\begingroup\$

I currently have this bit of code, but I need to repeat it for red green and blue. Is there a way I can do it without copying and pasting the code 3 times?

yellow.setOnClickListener(new View.OnClickListener() {    
    public void onClick (View v)   {   

        switch (buttonCount) {
        case 1:  
            empty1.setImageResource(R.drawable.yellow);
            buttonCount++;
            guess1= Colour.YELLOW;
            break;
        case 2:  
            empty2.setImageResource(R.drawable.yellow);
            buttonCount++;
            guess2=Colour.YELLOW;
            break;
        case 3:  
            empty3.setImageResource(R.drawable.yellow);
            buttonCount++;
            guess3=Colour.YELLOW;
            break;
        case 4:  
            empty4.setImageResource(R.drawable.yellow);
            buttonCount++;
            guess4=Colour.YELLOW;
            break;
        case 5:  
            empty5.setImageResource(R.drawable.yellow);
            buttonCount++;
            guess5=Colour.YELLOW;
            break;

        }
    }
});
\$\endgroup\$
1
  • 2
    \$\begingroup\$ Have you tried writing a function and then pass your arguments(Colour.RED, Color.BLUE and Color.GREEN). Remember that these are constants. Try it out and then post here what have you tried if you cannot achieve the outcome. \$\endgroup\$ – Maxood Nov 23 '13 at 20:17
11
\$\begingroup\$

If you look at your naming schema, you have emptyX and guessX, where X is an index. This is an indicator that you should be using an array or a list:

EmptyType empties[5] = { empty1, empty2, ... };
GuessType guesses[5] = { guess1, guess2, ... };

yellow.setOnClickListener(new View.OnClickListener() {
    public void onClick (View v) {
        assert buttonCount < 5;
        empties[buttonCount - 1].setImageResource(R.drawable.yellow);
        buttonCount++;
        guesses[buttonCount - 1] = Colour.YELLOW;
    }
})

If you have to do the same thing for three colours, either copy&paste this small snippet, or extract it into another method, which you can call with appropriate colours:

private void doStuff(EventSource source, Color color, ImageResource resource) {
  source.setOnClickListener(new View.OnClickListener() {
    public void onClick (View v) {
        assert buttonCount < 5;
        empties[buttonCount - 1].setImageResource(resource);
        buttonCount++;
        guesses[buttonCount - 1] = color;
    }
  });
}

  doStuff(yellow, Color.YELLOW, R.drawable.yellow);
  doStuff(blue,   Color.BLUE,   R.drawable.blue  );
  doStuff(green,  Color.GREEN,  R.drawable.green );

This still looks like you could choose better data structures, so that the Color and the corresponding ImageResource and EventSource are explicitly connected.

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