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, 2013 at 20:17

1 Answer 1

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 and acknowledge you have read our privacy policy.

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