patternjavaModerate
Best way to get rid of code repetition?
Viewed 0 times
ridwaygetrepetitioncodebest
Problem
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;
}
}
});Solution
If you look at your naming schema, you have
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:
This still looks like you could choose better data structures, so that the
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.Code Snippets
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;
}
})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 );Context
StackExchange Code Review Q#35986, answer score: 11
Revisions (0)
No revisions yet.