-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
java-racingcar(songpang) #3
base: songpang
Are you sure you want to change the base?
Changes from 46 commits
0a5d963
216cb6d
ae04e19
a6e8c0f
804fe2d
a02977d
3ac806d
a973407
78bb06d
bb9c87b
07f6628
cdfc826
0ff13cc
6519379
7aaac17
b115436
8d840f7
74d8054
02b5186
6954307
305ea0d
fc3b773
e0fe4fa
2d34588
e1c98f3
38feeaa
c47f82f
2c62d0c
838a38b
1a10b46
d9fbd3a
932e5fe
ed3d8ac
e0b9f8d
0b4dd36
7bb1ace
c7d7fd1
66dd3b6
a595c43
0852319
422c3da
8d80c12
645be80
302139e
93dde0d
bfb6277
b87577f
9a31667
51d46b3
5d7064e
b3444f0
978faee
eb18b53
021f98d
c296369
e577df7
560f251
8f84168
0428c22
ed92c1f
83eaf8c
54109bd
506b630
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
import domain.GamePlayer; | ||
|
||
public class Application { | ||
public static void main(String[] args){ | ||
GamePlayer gamePlayer = new GamePlayer(); | ||
gamePlayer.run(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,98 @@ | ||||||||||||
package domain; | ||||||||||||
|
||||||||||||
import io.Printer; | ||||||||||||
import io.Receiver; | ||||||||||||
|
||||||||||||
import java.util.ArrayList; | ||||||||||||
import java.util.List; | ||||||||||||
|
||||||||||||
public class GamePlayer { | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
클래스를 좀 더 분리해보는건 어떨까요? 🤔 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 대부분의 메소드의 접근 제한자가 default인데 의도하신 건가요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 처음에 public으로 설정했다가 동민님이 외부에서 사용하지 않는데 굳이 public을 사용한 이유가 있냐고 피드백을 주셔서 이에 대해 고민해 본 이후 default로 변경했습니다! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 아마 클래스 내부에서만 사용되는 메소드를 public으로 설정해서 그런 피드백을 주지 않았나 생각이 되네요 🙂 클래스 내부에서만 사용되는 메소드는 private, 외부에서 메세지를 전달받는 메소드는 public으로 설정하는 것이 좋을 것 같아요. |
||||||||||||
private static final int WINNER_CONDITION = 4; | ||||||||||||
|
||||||||||||
private final Printer printer; | ||||||||||||
private final Receiver receiver; | ||||||||||||
private final Generator generator; | ||||||||||||
|
||||||||||||
public GamePlayer() { | ||||||||||||
this.printer = new Printer(); | ||||||||||||
this.receiver = new Receiver(); | ||||||||||||
this.generator = new Generator(); | ||||||||||||
} | ||||||||||||
|
||||||||||||
void judgeAndMove(Car car, int randomNumber) { | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||||||||
if (randomNumber >= WINNER_CONDITION) | ||||||||||||
car.moveForward(); | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 핵데이 컨벤션 8.3 참고해주세요!
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 컨벤션을 확인해주세요 🙂 |
||||||||||||
} | ||||||||||||
|
||||||||||||
Car[] makeArrayAfterGettingName() { | ||||||||||||
printer.printGeneralMessage("INPUT_NAMEOFCAR"); | ||||||||||||
String[] listOfName = receiver.receiveName(); | ||||||||||||
|
||||||||||||
Car[] cars = new Car[listOfName.length]; | ||||||||||||
for (int i = 0; i < listOfName.length; i++) | ||||||||||||
cars[i] = new Car(listOfName[i]); | ||||||||||||
|
||||||||||||
return cars; | ||||||||||||
} | ||||||||||||
|
||||||||||||
int makeCountAfterGettingNumber() { | ||||||||||||
printer.printGeneralMessage("INPUT_COUNT"); | ||||||||||||
return receiver.receiveNumber(); | ||||||||||||
} | ||||||||||||
|
||||||||||||
void launchAllRound(Car[] cars, int countRound) { | ||||||||||||
for (int i = 0; i < countRound; i++) { | ||||||||||||
for (Car car : cars) { | ||||||||||||
judgeAndMove(car, generator.generateRandomNumber()); | ||||||||||||
printer.printProgressWithSymbol(car.getName(), car.getPosition()); | ||||||||||||
} | ||||||||||||
printer.printGeneralMessage("DEFAULT_SPACE"); | ||||||||||||
} | ||||||||||||
} | ||||||||||||
|
||||||||||||
List<Car> checkWhoIsWinner(Car[] cars) { | ||||||||||||
List<Car> winner = new ArrayList<>(); | ||||||||||||
int maxNumber = 0; | ||||||||||||
|
||||||||||||
for (Car car : cars) { | ||||||||||||
if(car.isMaxNumber(maxNumber)){ | ||||||||||||
winner.add(car); | ||||||||||||
} | ||||||||||||
if (car.isOverMaxNumber(maxNumber)) { | ||||||||||||
maxNumber = initWinner(winner, car); | ||||||||||||
} | ||||||||||||
} | ||||||||||||
|
||||||||||||
return winner; | ||||||||||||
} | ||||||||||||
|
||||||||||||
private int initWinner(List<Car> winner, Car car) { | ||||||||||||
int maxNumber = car.getPosition(); | ||||||||||||
winner.clear(); | ||||||||||||
winner.add(car); | ||||||||||||
|
||||||||||||
return maxNumber; | ||||||||||||
} | ||||||||||||
|
||||||||||||
public String makeWinnerToString(List<Car> cars) { | ||||||||||||
String winner = cars.get(0).getName(); | ||||||||||||
|
||||||||||||
if(cars.size() > 1) { | ||||||||||||
for(int i = 1;i<cars.size();i++) { | ||||||||||||
winner += ", " + cars.get(i).getName(); | ||||||||||||
} | ||||||||||||
} | ||||||||||||
|
||||||||||||
return winner; | ||||||||||||
} | ||||||||||||
public void run() { | ||||||||||||
Car[] cars = makeArrayAfterGettingName(); | ||||||||||||
int countRound = makeCountAfterGettingNumber(); | ||||||||||||
|
||||||||||||
printer.printGeneralMessage("OPERATION_RESULT"); | ||||||||||||
launchAllRound(cars, countRound); | ||||||||||||
|
||||||||||||
List<Car> winner = checkWhoIsWinner(cars); | ||||||||||||
printer.printWinner(makeWinnerToString(winner)); | ||||||||||||
} | ||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
package domain; | ||
|
||
public class Generator { | ||
private final int MAX_LIMIT_NUMBER = 10; //default value | ||
public int generateRandomNumber() { | ||
return (int) ((Math.random() * 10000) % MAX_LIMIT_NUMBER); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. JAVA API의 Random 클래스를 사용하면 난수 생성을 쉽게 할 수 있습니다. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 굳이 Random클래스를 사용하여 객체를 추가적으로 생성해야 한다는 것에 의문을 품어 Random 클래스를 사용하지 않았는데 지금와서 생각해보니 성능차이도 없거니와 모두가 주로 사용하는 클래스를 사용해야 가독성이 높아진다는 것을 깨달았습니다. |
||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,106 @@ | ||||||
package domain; | ||||||
|
||||||
import io.Printer; | ||||||
|
||||||
import java.util.Arrays; | ||||||
import java.util.List; | ||||||
import java.util.regex.Pattern; | ||||||
|
||||||
public class Validator implements ValidatorInterface { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validator을 따로 선언하여 입력 검증을 하는 책임을 분리하셨군요! 👍 하지만 ValidatorInterface의 추상화의 목적은 무엇인가요? 저도 저번 미션에서 받은 리뷰 내용이지만, 하지만 모든 경우에 있어 과도한 확장을 고려한다면 오히려 필요 이상으로 코드의 양이 많아집니다. 요구사항에 맞춘 추상화 수준으로 작성하고 요구사항의 변경이 있을 때 추상화하셔도 괜찮습니다. 😀 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 사실 이번 과제에서 인터페이스를 사용한 목적은 (미션 내내 페어프로그래밍을 할 수 없기 때문에) 기본 설계구조를 만들어 놓고 각 클래스 내부에 어떤 함수, 변수 등을 사용할 지 선언을 한 다음 따로 코드를 구현해와 비교하기 위해서 사용했습니다. 참고했던 인터페이스 관련 글 이 글을 읽고 협업시 인터페이스를 사용하면 더 편하겠다는 생각을 했어요. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 말씀하신 내용을 제가 제대로 이해했다면 객체는 행위 위주로 설계하는 것이 옳은 방식이라 이러한 의도에 있어서 인터페이스를 활용했다면 잘하신 일이라고 말씀드리고 싶습니다. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validator의 용도가 입력을 검증하고 오류 코드를 반환하는 것으로 느껴졌습니다. (물론 오류 코드보다는 true/false가 명확하기는 합니다) 그런 이유로 오류가 발생하는 그 즉시 오류를 검증하는 해당 메서드 내에서 메시지를 출력하는 결과로 나타난 것으로 보입니다. [클린코드 - 오류보다는 예외를 사용하라 참고] 또한 이러한 이유로 테스트 코드에서 Validator 테스트의 결과 검증을 true/false로만 할 수 밖에 없는데 예외적인 상황에서는 사용자 정의 예외 클래스를 작성해서 예외 처리를 해보세요 😸 |
||||||
private static final int MAX_NAME_SIZE = 5; | ||||||
private static final String valiNumber = "^[0-9]+$"; | ||||||
private static final String commaInARow = "^.*(,,).*+$"; | ||||||
private static final String characterOTN = "^[a-zA-Z,]+$"; | ||||||
songpang marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
Printer printer; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 접근 제한자가 필요할것 같아요! |
||||||
|
||||||
public Validator() { | ||||||
printer = new Printer(); | ||||||
} | ||||||
|
||||||
@Override | ||||||
public boolean validateName(String s) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 약어를 사용하기보단 적절한 이름을 사용해주세요! |
||||||
return inputNothing(s) | ||||||
&& inputCommaInARow(s) | ||||||
&& inputCharactersOtherThanName(s) | ||||||
&& startWithComma(s) | ||||||
&& endWithComma(s) | ||||||
&& overSizeCharacters(s) | ||||||
&& inputSameName(s); | ||||||
} | ||||||
|
||||||
@Override | ||||||
public boolean validateNumber(String s) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. boolean을 반환하는 함수의 네이밍은 일반적으로 is, can, should, has 등을 사용합니다.
Suggested change
만으로도 반환 타입이 무엇인지 나타낼 수 있으니 더 좋지 않을까요? |
||||||
return s.matches(valiNumber); | ||||||
} | ||||||
|
||||||
@Override | ||||||
public boolean inputNothing(String s) { | ||||||
if (s.equals("")) { | ||||||
printer.printExceptionMessage("INPUT_NOTHING"); | ||||||
return false; | ||||||
} | ||||||
return true; | ||||||
} | ||||||
|
||||||
@Override | ||||||
public boolean inputCommaInARow(String s) { | ||||||
if (Pattern.matches(commaInARow, s)) { //체크 필요 | ||||||
printer.printExceptionMessage("INPUT_COMMA_IN_A_ROW"); | ||||||
return false; | ||||||
} | ||||||
return true; | ||||||
} | ||||||
|
||||||
@Override | ||||||
public boolean startWithComma(String s) { | ||||||
if (s.charAt(0) == ',') { | ||||||
printer.printExceptionMessage("START_WITH_COMMA"); | ||||||
return false; | ||||||
} | ||||||
return true; | ||||||
} | ||||||
|
||||||
@Override | ||||||
public boolean endWithComma(String s) { | ||||||
if (s.charAt(s.length() - 1) == ',') { | ||||||
printer.printExceptionMessage("END_WITH_COMMA"); | ||||||
return false; | ||||||
} | ||||||
return true; | ||||||
} | ||||||
|
||||||
@Override | ||||||
public boolean inputCharactersOtherThanName(String s) { | ||||||
if (!Pattern.matches(characterOTN, s)) { | ||||||
printer.printExceptionMessage("INPUT_CHARACTERS_OTHER_THAN_NAME"); | ||||||
return false; | ||||||
} | ||||||
return true; | ||||||
} | ||||||
|
||||||
@Override | ||||||
public boolean inputSameName(String s) { | ||||||
List<String> CAR_NAME_LIST = Arrays.asList(s.split(",")); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 대문자와 언더스코어 조합의 네이밍은 상수 한에서만 적용되는 컨벤션입니다. |
||||||
int sizeOfNameList = CAR_NAME_LIST.size(); | ||||||
|
||||||
for (int i = 0; i < sizeOfNameList; i++) { | ||||||
if (CAR_NAME_LIST.subList(i + 1, sizeOfNameList).contains(CAR_NAME_LIST.get(i))) { | ||||||
printer.printExceptionMessage("INPUT_SAME_NAME"); | ||||||
return false; | ||||||
} | ||||||
} | ||||||
return true; | ||||||
} | ||||||
|
||||||
@Override | ||||||
public boolean overSizeCharacters(String s) { | ||||||
String[] splitName = s.split(","); | ||||||
for (String i : splitName) | ||||||
if (i.length() > MAX_NAME_SIZE) { | ||||||
printer.printExceptionMessage("OVER_SIZE_CHARACTERS"); | ||||||
return false; | ||||||
} | ||||||
return true; | ||||||
} | ||||||
|
||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
package domain; | ||
|
||
public interface ValidatorInterface { | ||
boolean validateName(String s); | ||
|
||
boolean validateNumber(String s); | ||
|
||
boolean inputNothing(String s); | ||
|
||
boolean inputCommaInARow(String s); | ||
|
||
boolean startWithComma(String s); | ||
|
||
boolean endWithComma(String s); | ||
|
||
boolean inputCharactersOtherThanName(String s); | ||
|
||
boolean inputSameName(String s); | ||
|
||
boolean overSizeCharacters(String s); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
package io; | ||
|
||
import java.util.HashMap; | ||
import java.util.Map; | ||
|
||
public class Message implements MessageInterface { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 출력 메시지를 한곳에 몰아서 작성하셨네요! 읽기 편한 것 같습니다 👍 다만 사용할 수 있는 메시지의 종류를 열거형으로 정의하면 Message 객체를 사용하는 Validator에서 |
||
Map<String, String> generalMessageList = new HashMap<String, String>(); | ||
Map<String, String> exceptionMessageList = new HashMap<String, String>(); | ||
|
||
public Message() { | ||
this.exceptionMessageList(); | ||
this.generalMessageList(); | ||
} | ||
private void generalMessageList() { | ||
generalMessageList.put("INPUT_NAMEOFCAR", "경주할 자동차 이름을 입력하세요. (이름은 쉼표(,) 기준으로 구분)"); | ||
generalMessageList.put("INPUT_COUNT", "시도할 회수는 몇회인가요?"); | ||
generalMessageList.put("OPERATION_RESULT", "실행 결과"); | ||
generalMessageList.put("FIANL_WINNER", "가 최종 우승했습니다."); | ||
generalMessageList.put("DEFAULT_SPACE", ""); | ||
} | ||
|
||
private void exceptionMessageList() { | ||
exceptionMessageList.put("INPUT_NOTHING", "아무 문자도 입력되지 않았습니다."); | ||
exceptionMessageList.put("INPUT_COMMA_IN_A_ROW", "쉼표(,)가 연속으로 입력되었습니다."); | ||
exceptionMessageList.put("INPUT_CHARACTERS_OTHER_THAN_NAME", "영어와 쉼표(,) 이외의 다른 문자가 입력되었습니다."); | ||
exceptionMessageList.put("START_WITH_COMMA", "쉼표(,)로 시작합니다."); | ||
exceptionMessageList.put("END_WITH_COMMA", "쉼표(,)로 끝납니다."); | ||
exceptionMessageList.put("INPUT_SAME_NAME", "같은 이름이 입력되었습니다."); | ||
exceptionMessageList.put("OVER_SIZE_CHARACTERS", "이름이 5자 이상입니다."); | ||
exceptionMessageList.put("INPUT_CHARACTERS_OTHER_THAN_DIGIT", "숫자 이외의 다른 문자가 입력되었습니다."); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 메세지를 Map으로 관리하려는 의도는 무엇이었나요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 첫번째는 Key값을 통해 메세지들을 관리하면 효율적일 것 같다는 생각에서였고 이를 통해 클래스를 만들어 관리하면 일괄적으로 관리할 수 있다는 이점이 추가적으로 있을 것 같다는 생각이었습니다! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 만약 key값을 통해 관리하게 된다면 |
||
|
||
@Override | ||
public String selectMessageFromGeneral(String Keyword) { | ||
return generalMessageList.get(Keyword); | ||
} | ||
|
||
@Override | ||
public String selectMessageFromException(String Keyword) { | ||
return exceptionMessageList.get(Keyword); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
run을 제외한 모든 메서드들이 외부에서 호출될 필요가 없어보입니다.
public 접근 제어자로 설정하는 것이 괜찮을까요?