-
Notifications
You must be signed in to change notification settings - Fork 1
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
[switch week4] junghyun-eunjung #11
base: main
Are you sure you want to change the base?
Conversation
private boolean isValidPairs(List<PairInfoJava> newPairs, List<List<PairInfoJava>> pairHistory) { | ||
if (pairHistory.isEmpty()) return true; | ||
|
||
// todo: 이번주 안으로.. ^.^ 검증 로직까지 완료할게요 . . . | ||
return true; |
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.
@parkje0927 sorry . . . 너무 하기 싫어서 . . . 이번주 안으로 마무리할게요 요 부분 .. .
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.
너무너무너무너무 고생 많으셨습니다~
코멘트들은 그냥 혹시나 나중에 자바에 필요한 내용이 될 것 같아 작성한 내용이고, 그냥 읽어만 주시면 될 것 같습니당
이미 추상화나 Stream, record 객체 활용 등 여러 고민들을 많이 해주셔서 너무 재밌게 코멘트 작성하고 마무리했습니다~~🙂 멋짐 그 자체
@@ -0,0 +1,17 @@ | |||
import java.util.Scanner; | |||
|
|||
public abstract class BaseInterpreter<T> { |
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.
오 추상 클래스를 사용하셨군요~
출력 내용들이 반복적이라 고민이 되는 부분들이 있었을텐데 추상화 개념들을 활용해봐서 좋은 것 같습니다👍👍
/** | ||
* getter 자동 생성해주는 레코드 객체 - 불변객체이다. | ||
*/ | ||
public record CommandDetailJava( |
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.
record 객체를 사용하셨군요~~ 👍
|
||
CourseJava(String inputCode) { | ||
this.inputCode = inputCode; | ||
this.printingName = this.inputCode; |
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.
inputCode, printingName 중에 하나만 있어도 될 것 같아요~
LEVEL2("레벨2", new String[]{"장바구니", "결제", "지하철노선도"}), | ||
LEVEL3("레벨3", new String[]{}), | ||
LEVEL4("레벨4", new String[]{"성능개선", "배포"}), | ||
LEVEL5("레벨5", new String[]{}); |
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.
은정님 코드 보니까 Level 에다가 활동을 넣었어도 좋았겠다는 생각이 들었습니당 그 레벨에 묶인 거니까 같이 표현해도 좋았을 것 같고, 활동들에 대해서도 enum 으로 정의해서 여기에 같이 표현해줬어도 좋았을 것 같아요~
public enum Activity {
RACING("자동차경주"),
LOTTO("로또"),
BASEBALL_GAME("숫자야구게임");
private final String name;
Activity(String name) {
this.name = name;
}
}
public enum Level {
LEVEL1("레벨1", List.of(Activity.RACING)),
LEVEL2("레벨2", List.of(Activity.LOTTO, Activity.BASEBALL_GAME));
}
} | ||
|
||
@Override | ||
protected CommandDetailJava parser(String input) { |
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.
자바에서는 null 을 반환하는 것은 지양하고 있는데 null 처리를 잘 해주지 못하면 다른 곳에서 npe 가 발생해서 오류가 전파가 되기도 하고 여기 함수에서 책임을 가지고 해당 변수에 대해 체크를 해야하는 부분을 넘겨버리는 거라고도 생각을 해서 그렇습니당
저라면 그냥 exception 을 던져서 프로그램을 종료시켜버릴 것 같긴 한데ㅋㅋㅋ (뭐야 이 이상한 데이터는ㅡㅡ 종료.👾)
좀 더 자연스럽게 처리하고 싶다면 Optional 객체를 써도 좋을 것 같습니다.
@Override
protected Optional<CommandDetailJava> parser(String input) {
String[] inputs = input.split(",");
if (inputs.length < 2 || inputs.length > 3) {
return Optional.empty();
}
CourseJava course = CourseJava.parseFrom(inputs[0].trim());
LevelJava level = LevelJava.parseFrom(inputs[1].trim());
String activity = inputs.length > 2 ? inputs[2].trim() : null;
if (course == null || level == null) {
return Optional.empty();
}
if (isValidActivity(level, activity)) {
return Optional.of(new CommandDetailJava(level, course, activity));
}
return Optional.empty();
}
이렇게 Optional 로 감싸서 반환을 하고,
이 함수를 사용하는 곳에서 isPresent(): boolean
를 활용해서 존재하면 get()
해서 사용하는 식으로 해도 괜찮았을 것 같아요~
pairHistory.clearHistory(); | ||
System.out.println("초기화 되었습니다 ! ! !"); | ||
} | ||
|
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.
나~~~중에 한번 show, match, clear 함수들을 클래스로 다 분리해서 그 안에서 수행되는 메소드로 가져가면 자바 그냥 마스터 하실 것 같습니다🤍
week 4