-
Notifications
You must be signed in to change notification settings - Fork 55
[BCSD Lab] 박태진, 4차시 미션 제출합니다 #74
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
Conversation
<기능 요약> 크기가 고정된 사다리 타기 게임 생성 <기능 상세> - App > RendomConnectionGenerator > LadderRender 순서로 흐름 - 크기는 4*4로 고정되어 있음. - 한 행에서, 이전 행이 연결되어 있다면 무조건 다음 행은 연결되지 않도록 구현 - 추후 크기가 변하면 유연하게 대처할 수 있도록 구성
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.
마지막 미션까지 고생하셨습니다🥳
코드에 대한 제 생각 남겨놨습니다.
src/main/java/App.java
Outdated
boolean allValid = names.stream().allMatch(n -> n.length() <= 5); | ||
if (!allValid) { |
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.
저는 이런 로직이 나올때 따로 함수로 allValid(names) 처럼 만들어서 사용하는 편이에요.
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.
오!! 조언해주셔서 감사합니다~!😀 제가 작성한 코드를 함수로 분리하여 작성하면 더 가독성이 높아질 것 같습니다.
다음엔 함수로 분리하여 작성해보도록 하겠습니다!!
src/main/java/App.java
Outdated
String who = scanner.nextLine().trim(); | ||
System.out.println(); | ||
System.out.println("실행 결과"); | ||
if (who.equalsIgnoreCase("all")) { |
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.
오 equalsIgnoreCase
메서드라는게 있었네요 새로 배워갑니다
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.
수고하셨습니다!
코드에 정답은 없으니 전부 받아들이지 않아도 됩니다! 다른 의견이 있으시다면 편하게 말해주세요 :)
import view.LadderRenderer; | ||
|
||
public class App { | ||
public static void main(String[] args) { |
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.
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.
아차차... 알려주셔서 감사합니다🙇
바로 수정하도록 하겠습니다!!
@@ -0,0 +1,30 @@ | |||
package domain; | |||
|
|||
public final class Columns { |
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.
final class로 만든 이유가 있을까요?
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.
열의 개수는 최소 2개라는 불변 조건을 지켜야 하므로, 이 조건을 항상 만족할 수 있도록 상속을 막는 final을 사용했습니다!
|
||
public static Columns fixedFour() { | ||
return of(4); | ||
} |
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.
사다리 크기를 입력 받아 생성하기 전에 쓰던 코드가 남아있는 것 같아요.
지금처럼 상수 4
로 Columns를 생성하는 경우, 이 상수는 누구의 책임일까요? 상수를 통한 생성이 Columns에 있는게 맞을까요?
상수를 이용해 Columns를 구성할 경우, 해당 상수는 Columns를 생성하는 주체 측에 위치하는 것이 바람직하다고 생각해요. 테스트 코드를 작성하면서 fixture를 정의할 때 Columns 클래스 내부에 fixture를 작성하지 않는 것처럼요.
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.
말씀해 주신 것처럼 Columns 에서 검증만 맡고 4같은 상수는 생성하는 주체에 있는 것이 올바른 것 같습니다.
상수 4로 columns를 생성하면 이 상수는 상수 4를 생성하라고 호출한 생성 주체가 책임이라고 생각합니다.
src/main/java/domain/Rows.java
Outdated
public static Rows fixedFour() { | ||
return of(4); | ||
} |
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.
사다리 크기를 입력 받아 생성하기 전에 쓰던 코드가 남아있는 것 같아요.
지금처럼 상수4
로 Columns를 생성하는 경우, 이 상수는 누구의 책임일까요? 상수를 통한 생성이 Columns에 있는게 맞을까요?상수를 이용해 Columns를 구성할 경우, 해당 상수는 Columns를 생성하는 주체 측에 위치하는 것이 바람직하다고 생각해요. 테스트 코드를 작성하면서 fixture를 정의할 때 Columns 클래스 내부에 fixture를 작성하지 않는 것처럼요.
동일
public static RungLength defaultFive() { | ||
return of(5); | ||
} |
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.
사다리 크기를 입력 받아 생성하기 전에 쓰던 코드가 남아있는 것 같아요.
지금처럼 상수4
로 Columns를 생성하는 경우, 이 상수는 누구의 책임일까요? 상수를 통한 생성이 Columns에 있는게 맞을까요?상수를 이용해 Columns를 구성할 경우, 해당 상수는 Columns를 생성하는 주체 측에 위치하는 것이 바람직하다고 생각해요. 테스트 코드를 작성하면서 fixture를 정의할 때 Columns 클래스 내부에 fixture를 작성하지 않는 것처럼요.
동일
src/main/java/domain/Rows.java
Outdated
@@ -0,0 +1,26 @@ | |||
package domain; | |||
|
|||
public final class Rows { |
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.
Rows
이름만 보면 내부적으로 Row
리스트를 가지고 있을 것 같은데 상수 하나만 존재하는게 살짝 어색해요.
Rows 말고 다른 이름은 어떨까요?
아니면 Rows 내부적으로 Row를 가지고 길이나 갯수가 필요할 경우 리스트의 size를 반환하는건 어떨까요?
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.
지적해주셔서 감사합니다 🙇
Rows라는 이름이 Row의 컬렉션으로 오인될 수 있어, 그 의미를 명확히 하고자 LadderHeight로 수정해보겠습니다!
@@ -0,0 +1,30 @@ | |||
package domain; | |||
|
|||
public final class Columns { |
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.
Rows
이름만 보면 내부적으로Row
리스트를 가지고 있을 것 같은데 상수 하나만 존재하는게 살짝 어색해요.
Rows 말고 다른 이름은 어떨까요?
아니면 Rows 내부적으로 Row를 가지고 길이나 갯수가 필요할 경우 리스트의 size를 반환하는건 어떨까요?
동일
private final List<Connection> connections; | ||
|
||
private Row(List<Connection> connections) { | ||
this.connections = Collections.unmodifiableList(new ArrayList<>(connections)); |
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.
이런게 있었군요! 자세한 내용이 궁금해서 찾아봤어요!
개인적으로 배열을 복사해서 unmodifiable하게 만드는 대신에 배열을 복사해서 캡슐화를 잘하면 되지 않을까라는 생각이 드는 것 같아요. 태진님의 의견이 궁금합니다!
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.
캡슐화를 진행해도 좋을 것 같습니다!! 😀
코드 작성 당시, 저 방식이 성능과 안전성을 모두 고려한 좋은 방법이라고 생각했었습니다. 생성 시 한 번만 방어적 복사를 해두면 잦은 호출에도 성능 부담이 적고, 불변인 enum 요소를 변경 불가 뷰로 제공하니 데이터 안정성도 충분히 확보된다고 보았습니다.
src/main/java/App.java
Outdated
public class App { | ||
public static void main(String[] args) { | ||
try (Scanner scanner = new Scanner(System.in)) { | ||
List<String> names; |
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.
사람 정보도 클래스로 분리할 수 있을 것 같아요!
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.
오!!!!! 알려주셔서 감사합니다😀✨
클래스로 분리하는 것이 더 깔끔하고 좋을 것 같다고 생각이들어서, 지금 바로 수정해보겠습니다!!
조언해주셔서 감사합니다~!!
안녕하세요?!
4차 미션 제출합니다.