몇달전에 만들어놓았던 숫자야구 프로젝트가 있었다. 국비학원 초창기에 Linear Layout 만 배웠을 때 과제로 만들었던 게임이었는데, 좀 더 깔끔한 코드로 리팩토링 해봐야지 해봐야지 하다가 이제서야 시작해본다.
일단은 findViewById() 로 찾아오던 View 들을 ViewBinding 을 이용한 코드로 바꿔보려 한다.
여기저기 코드를 들쑤시기 보다는 예전에 어떻게 코드를 짰었는지 복기도 해볼겸, 로직에 따라 차근차근 바꿔보려 한다.
그럼 시작!
초기화 코드 부분
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
binding = ActivityMainBinding.inflate(LayoutInflater.from(this));
setContentView(binding.getRoot());
initial();
}
private void initial(){
// 뷰 객체 id 대입
num1 = findViewById(R.id.num1);
num2 = findViewById(R.id.num2);
num3 = findViewById(R.id.num3);
strike = findViewById(R.id.strike);
ball = findViewById(R.id.ball);
out = findViewById(R.id.out);
gameResult = findViewById(R.id.gameResult);
ballResult2 = findViewById(R.id.ballResult2);
round = findViewById(R.id.round);
round.setText( roundCount +"");
round.setVisibility(View.VISIBLE);
// 컴퓨터 정답 만들기
setAnswer();
// 버튼 리스너
btnArr = new Button[]{binding.btn0, binding.btn1, binding.btn2, binding.btn3, binding.btn4, binding.btn5, binding.btn6, binding.btn7, binding.btn8, binding.btn9};
for (Button button : btnArr) button.setOnClickListener(listener);
binding.btnRetry.setOnClickListener((v)->retryEvent());
}
일단 initial() 메소드를 만들어서 onCreate() 에 몰려있던 코드를 좀 빼놨다. 나는 onCreate() 에 코드가 복잡하게 쓰여져 있는게 별로 보기 안좋아서 초기화 작업용 메소드를 따로 만들어주었다. 로직을 따라가려면 setAnswer() 부터 시작해야겠다. findViewById() 부분은 어차피 나중에 삭제될 코드니까..
그럼 먼저 setAnswer() 부분부터 보자.
숫자야구 정답 만드는 부분
public void setAnswer(){
answer100 = rand.nextInt(10);
while(answer100 == answer10) answer10 = rand.nextInt(10);
while(answer100 == answer1 || answer10 == answer1) answer1 = rand.nextInt(10);
}
숫자야구 게임에서 게임이 진행되려면 맞춰야 하는 숫자가 있어야 한다. 그 숫자를 지정해주는 코드이다. Random 클래스를 이용하여 숫자를 무작위로 작성해주었다.
public void setAnswer(){
Random rand = new Random();
answer100 = rand.nextInt(10);
while(answer100 == answer10) answer10 = rand.nextInt(10);
while(answer100 == answer1 || answer10 == answer1) answer1 = rand.nextInt(10);
}
ViewBinding 관련해서는 따로 변경할 부분은 없는것 같다. 다만, 다른 메서드에서 사용하지도 않는데 rand 변수가 클래스 멤버변수로 설정되어있었다. 그래서 지역변수로 바꿔주고 setAnswer() 메소드 내에서만 사용할 수 있도록 변경. 또한 실제로 게임에서 정답을 맞춰보면 정답이 거진 비슷하게 설정되었었다. 분명 랜덤하게 돌린것같은데 왜 그런 현상이 나타나는지 모르겠지만, 그 부분은 추후에 다시 살펴보자..
숫자 버튼 리스너
그 다음은, 버튼 리스너 부분이다. 버튼 리스너는 다음 코드와 같다.
private final View.OnClickListener listener = (view)->{
setBtnNumber((Button) view);
allBtnChecked();
};
이 중에서 setBtnNumber() 부터 살펴보자.
public void setBtnNumber (Button btn){
if(!btn.isSelected()) {
if (num1.getText().equals("")) num1.setText(btn.getText());
else if (num2.getText().equals("")) num2.setText(btn.getText());
else num3.setText(btn.getText());
btn.setBackgroundColor(Color.parseColor("#FF555950"));
btn.setTextColor(Color.parseColor("#FF2C2B2B"));
btn.setSelected(true);
}else{
if (num3.getText().equals(btn.getText())) num3.setText("");
else if (num2.getText().equals(btn.getText())) num2.setText("");
else num1.setText("");
btn.setBackgroundColor(Color.parseColor("#9CB580"));
btn.setTextColor(Color.parseColor("#FFFFFF"));
btn.setSelected(false);
}
}
일단 메소드 이름이 걸린다. 버튼을 클릭할 때, 선택한 숫자를 전광판? 에 기입하는 코드인데, setBtnNumber 는 뭔가 이상하다. 그 이외에는 뭐.. viewBinding 으로만 대체하면 될것같다. 뭔가 예전에 짜놓은 코드라 그런지 껄쩍지근한 느낌이 들긴하지만 일단 그 부분만 변경해보자.
public void setSelectNumber (Button btn){
if(btn.isSelected()) {
if (binding.num3.getText().equals(btn.getText())) binding.num3.setText("");
else if (binding.num2.getText().equals(btn.getText())) binding.num2.setText("");
else binding.num1.setText("");
btn.setBackgroundColor(Color.parseColor("#9CB580"));
btn.setTextColor(Color.parseColor("#FFFFFF"));
btn.setSelected(false);
}else{
if (binding.num1.getText().equals("")) binding.num1.setText(btn.getText());
else if (binding.num2.getText().equals("")) binding.num2.setText(btn.getText());
else binding.num3.setText(btn.getText());
btn.setBackgroundColor(Color.parseColor("#FF555950"));
btn.setTextColor(Color.parseColor("#FF2C2B2B"));
btn.setSelected(true);
}
}
숫자 선택 완료 코드 부분
그럼 이제 allBtnChecked() 를 살펴볼 차례다. 일단 메소드 이름만 봐서는 모든 버튼들이 선택되었을 때 처리하는 코드인듯하다.
public void allBtnChecked(){
int count = 0;
for(Button b : btnArr) if(b.isSelected()) count++;
if(count == 3){
for(int i = 0; i < 10; i++){
btnArr[i].setSelected(false);
}
inputNumber();
init();
}
}
일단 보기에는 count 변수를 설정해둔게 맘에 안든다. 아마 원래 로직은 모든 버튼들을 for 문을 통해 살펴보면서, 선택된게 3개라면 모두 선택된거니, 다음 코드를 진행시키는 로직인것같다. 그거보다는 전광판 TextView 를 돌면서 빈 문자열이 나타나면 allBtnChecked() 를 return 시키는게 나아보인다. 빈 문자열이 없다면 다 선택된거니까.
그리고 이름좀 바꿔보았다.
public void selectedAll(){
if(binding.num1.getText().equals("")
|| binding.num2.getText().equals("")
|| binding.num3.getText().equals("") ) return;
for(int i = 0; i < 10; i++) btnArr[i].setSelected(false);
inputNumber();
init();
}
이렇게 하고 실행을 시켜보니, 숫자 3개를 다 클릭했더니 앱이 다운되었다.. ㅠㅠ 왜그런지 살펴봤더니, 내가 아직까지 건들지 않은 코드 부분에서 에러가 났다. 에러를 살펴보니 그냥 실수였다. xml 에 id 가 맘에 안들어서 살짝 바꿔줬는데 다른 TextView 에 id 를 바꾸는 바람에 NPE 가 났다. 성급하게 이것저것 바꾸다보니 이런 실수가 나온다. 좀 차근차근 바꾸자.
strike, ball, out 체크 코드 부분
에러도 해결했고, 이제 그러면 로직을 따라가보면 이제 inputNumber() 를 살펴봐야할 차례. 이름만 봐서는 숫자를 넣는다? 이런 뜻인데, 코드를 살펴보니 strike 랑 ball, out 을 체크하는 로직이 들어있었다. 이름이 단단히 잘못된것같다. 이름부터 바꾸자.
public void compareToAnswer(){
String s1 = num1.getText().toString();
int n100 = Integer.parseInt(s1);
String s2 = num2.getText().toString();
int n10 = Integer.parseInt(s2);
String s3 = num3.getText().toString();
int n1 = Integer.parseInt(s3);
if(n100 == answer100) strikeNum++;
else if(n100 == answer10) ballNum++;
else if(n100 == answer1) ballNum++;
else outNum++;
if(n10 == answer100) ballNum++;
else if(n10 == answer10) strikeNum++;
else if(n10 == answer1) ballNum++;
else outNum++;
if(n1 == answer100) ballNum++;
else if(n1 == answer10) ballNum++;
else if(n1 == answer1) strikeNum++;
else outNum++;
strike.setText(String.valueOf(strikeNum));
ball.setText(String.valueOf(ballNum));
out.setText(String.valueOf(outNum));
homeRun();
}
일단 코드를 보고 든 생각은 코드가 길다는 생각이다. 전광판 숫자를 가져와서 String 으로 바꿔주는 부분은 따로 변수에 넣지말고 바로 바꿔주는게 간결할것같고, viewBinding 적용할 부분 적용하고, homerun() 메소드는 살펴봐야할것같다.
public void compareToAnswer(){
int n100 = Integer.parseInt(num1.getText().toString());
int n10 = Integer.parseInt(num2.getText().toString());
int n1 = Integer.parseInt(num3.getText().toString());
if(n100 == answer100) strikeNum++;
else if(n100 == answer10 || n100 == answer1) ballNum++;
else outNum++;
if(n10 == answer10) strikeNum++;
else if(n10 == answer100 || n10 == answer1) ballNum++;
else outNum++;
if(n1 == answer1) strikeNum++;
else if(n1 == answer10 || n1 == answer100) ballNum++;
else outNum++;
strike.setText(String.valueOf(strikeNum));
ball.setText(String.valueOf(ballNum));
out.setText(String.valueOf(outNum));
homeRun();
}
homerun() 부분은 strike 숫자를 체크해서 3 이라면 정답을 맞춘게 되므로, 게임을 끝내는 코드였다. 따로 건들건 없었고, viewBinding 만 적용시켜줬다.
public void homeRun(){
if(strikeNum != 3) return;
for(int i = 0; i < 10; i++){
btnArr[i].setEnabled(false);
btnArr[i].setBackgroundColor(Color.parseColor("#FF555950"));
btnArr[i].setTextColor(Color.parseColor("#FF2C2B2B"));
}
binding.gameResult.setVisibility(View.VISIBLE);
binding.btnRetry.setVisibility(View.VISIBLE);
}
이제 다시 돌아와서 init() 을 볼 차례. 일단 init 을 메소드 이름으로 쓰는건 별로 좋지 않다. 예약어 였던가 그랬었던것 같다. 코드를 보니 다음 라운드로 넘어갈 때 필요한 작업 코드가 있었다. 그래서 nextRound() 로 이름 변경. 그리고 이 nextRound() 가 selectedAll() 메소드 내부에서 실행되고 있는데, 이것보다는정답을 체크한 뒤, strike 가 3이라면 homerun() 을, 아니라면 nextRound() 를 실행시키는것이 좋아보인다. 이제 nextRound() 내부 코드를 변경해보자.
다음 라운드 진행 코드
public void nextRound(){
if(strikeNum != 3) {
roundCount++;
round.setText(roundCount + "");
}
// 버튼 초기화
if(strikeNum != 3) {
for (int i = 0; i < 10; i++) {
btnArr[i].setEnabled(true);
btnArr[i].setBackgroundColor(Color.parseColor("#9CB580"));
btnArr[i].setTextColor(Color.parseColor("#FFFFFF"));
}
}
// 선택결과 저장을 위해 이전 결과들을 화면에 표기
drawNum();
// 선택 숫자 초기화
num1.setText("");
num2.setText("");
num3.setText("");
// 새로운 선택결과와 정답의 비교를 위해 s,b,o 초기화
strikeNum = 0;
ballNum = 0;
outNum = 0;
}일단 nextRound() 가 실행되었다는건 strike 가 3이 아니라는 이야기 이므로, 맨 위에 적혀있는 if 문은 필요없어보인다. 그리고 두번째 if 문은.. 왜 있었는지 모르겠다. 그것도 지워주자. 흠 그리고 초기화와 관련된 코드들이 많아보이는데, 따로 메소드로 묶을 수 있는지 나중에 한번 살펴봐야겠다. viewBinding 적용할 부분 적용하고, drawNum() 을 살펴보면 될듯 하다.
public void nextRound(){
roundCount++;
round.setText(String.valueOf(roundCount));
drawNum();
resetElement();
}
private void resetElement(){
for (int i = 0; i < 10; i++) {
btnArr[i].setEnabled(true);
btnArr[i].setBackgroundColor(Color.parseColor("#9CB580"));
btnArr[i].setTextColor(Color.parseColor("#FFFFFF"));
}
binding.num1.setText("");
binding.num2.setText("");
binding.num3.setText("");
strikeNum = 0;
ballNum = 0;
outNum = 0;
}
nextRound() 부분에 다음 라운드 진행을 위한 리셋 코드가 들어가 있어, 따로 메소드를 분류해 빼주었다. 나름 깔끔해진듯한 느낌. viewBinding 도 적용시켜 주었다. 여기까지 하고 다음 포스팅으로 넘기도록 하자.