사다리 미션 제출합니다 :) #18
사다리 미션 제출합니다 :) #18
Conversation
| Output.printParticipants(participants); | ||
|
|
||
| // 가로 사다리 배열 만들기 | ||
| ArrayList<ArrayList<Integer>> array = LadderArray.createLadderArray(participants.size()-1, height); |
There was a problem hiding this comment.
ArrayList< ArrayList> 를 사용하지 않게, LadderArray 클래스를 Ladder과 각 줄을 나타내는 Line 클래스로 나누어서 구현해도 좋을 것 같습니다!
1단계-사다리출력힌트의 내용을 참고하시면 될 것 같아요.
| // 검색 | ||
| private static void search(ArrayList<Participant> participants) { | ||
| // 결과 검색 | ||
| String search = Input.searchResult(); | ||
| if (search.equalsIgnoreCase("all")) { | ||
| // 모든 참가자의 결과 출력 | ||
| Output.printAllResults(participants); | ||
| } | ||
| if (! search.equalsIgnoreCase("all")) { | ||
| // 개별 참가자의 결과 출력 | ||
| Output.printResultForParticipant(participants, search); | ||
| } | ||
| } |
There was a problem hiding this comment.
출력하는 메서드는 Output 클래스로 옮기고, Application 클래스에는 controller의 역할만 수행하면 좋을 것 같습니다:)
| // 각 참가자의 최종 결과를 구하기 1 | ||
| public static void setResultsForParticipants(ArrayList<Participant> participants, ArrayList<String> results, ArrayList<ArrayList<Integer>> array) { | ||
| for (int i = 0; i < participants.size(); i++) { | ||
| int finalPosition = findFinalPosition(i, array); | ||
| participants.get(i).setResult(results.get(finalPosition)); | ||
| } | ||
| } | ||
|
|
||
| // 각 참가자의 최종 결과 구하기 2 | ||
| private static int findFinalPosition(int startIndex, ArrayList<ArrayList<Integer>> array) { | ||
| int position = startIndex; | ||
|
|
||
| for (ArrayList<Integer> row : array) { | ||
| position = findFinalPosition2(row, position); | ||
| } | ||
|
|
||
| return position; | ||
| } | ||
|
|
||
| // 각 참가자의 최종 결과 구하기 3 | ||
| private static int findFinalPosition2(ArrayList<Integer> row, int position){ | ||
| if (position > 0 && row.get(position - 1) == 1) { | ||
| position--; | ||
| return position; | ||
| } | ||
| if (position < row.size() && row.get(position) == 1) { | ||
| position++; | ||
| return position; | ||
| } | ||
| return position; | ||
| } |
There was a problem hiding this comment.
모든 엔티티를 작게 유지하기 위해, 최종 결과를 구하는 메서드들은 따로 새로운 클래스에 넣어도 될 것 같아요!
| public static ArrayList<ArrayList<Integer>> createLadderArray(int x, int y) { | ||
| System.out.println(); | ||
| ArrayList<ArrayList<Integer>> array2D = new ArrayList<>(); |
There was a problem hiding this comment.
위에서 말씀드렸던 것 처럼
// AS-IS
public class Ladder {
private final List<List<Boolean>> lines;
}
이렇게 이차원 배열을 사용하는 것 보다
// TO-BE
public class Ladder {
private final List<Line> lines;
}
public class Line {
private final List<Boolean> points;
}
이렇게 사용하면 좋을 것 같습니다!
There was a problem hiding this comment.
[Request Change]
위에서 리뷰 해주신 것처럼,
연결다리(Intger), 라인(List<Integer>), 사다리(List<List<Integer>>)가 각각 역할을 가질 수 있도록 클래스로 래핑해보면 어떨까요?
연결다리(Integer)
- 1일 때는 건널 수 있다.
- 0일 때는 건널 수 없다.
- 0과 1의 값만 가진다.
- 0과 1의 값중 랜덤하게 생성된다.
라인(List)
연결다리를 사람수만큼 갖는다.- 연속해서 건널 수 있는
연결다리는 가질 수 없다. - 참가자는 매 라인마다 건널 수있는
연결다리가 있으면 이동 / 없으면 이동 X
사다리(List<List>)
- 사다리 높이만큼
라인을 갖는다. - 높이는 2 이상이여야한된다.
- 참가자는 매 라인 이동이 끝나면 다음 라인으로 넘어간다.
이런 요구사항들을 도출해볼 수 있겠죠?
위의 예시는 제가 README를 작성한다면 뽑아내볼 요구사항을 간략히 적어본 것이고 저렇게 하라고 적은 것은 당연히 아닙니다.
아무래도 객체의 책임 분리에 대해서 어려움이 있으실 것 같아 예시를 남겨드린것이니,
준민님만의 요구사항을 도출해보고 객체를 나눠보는 것은 어떨까요?
- README를 통해 필요한 객체(모델, ...)의 역할 및 기능 나열
- README를 토대로 class 설계 및 작성
- 리팩터링하며, 책임분리 더 명확히 하기
이런 방식으로 연습해보면 좋을 것 같아요
There was a problem hiding this comment.
안녕하세요, 준민님~
리뷰어 루카입니다!!🐳
일단 빨리 리뷰를 남겨드리지 못해 죄송합니다.
알림이 온것을 보지 못했습니다..ㅠㅠ
그리고 제가 봤을 땐 소통을 더 자주 많이할수록 얻어가실 것이 많아 보입니다!!
이번에 다방면으로 리뷰를 남기기 보단 포인트가 될만한 것들을 남기겠습니다.
🌻 리뷰 종류
정확한 리뷰 전달을 위해 리뷰를 3가지로 나눴습니다.
[Request Change]
컨벤션에 어긋남
버그를 유발
꼭 학습이 필요한 부분
위와 같이 꼭 변경이 필요할 때 한 리뷰입니다.
[Comment]
더 나은 방법을 고민해보면 좋은 점
의도를 파악하기 힘든 부분
이와 같이 심각한 부분은 아니지만 생각 공유가 더 필요한 부분에 달았습니다.
[Approve]
정말 좋다고 생각하는 부분
너무 좋으나, 추가적인 생각을 달고 싶은 부분
🙆♀️ 리뷰 방향성
정답은 없습니다.
준민님의 코드도 매우 훌륭한 코드고 기능이 어느정도 잘 완성된 코드라고 생각합니다.
저희는 이 미션을 통해서 연습하는 것들 중에 큰 맥락이 클린 코드인데요.
왜 클린 코드를 해야할까요?
제가 준민님 코드를 봤을 때 준민님은 백준 알고리즘 같은 수학적인 문제를 잘 푸실것 같아요.
백준에 문제 제출을 할 때 클린한 코드가 점수에 반영되진 않죠.
하지만, 현실에서 프로덕트를 만들때는 그 프로덕트는 여러 팀원에 의해서 그 사업의 수명동안 유지보수되어야합니다.
그래서 클린 코드는 유지보수와 협업이 용이한 가치가 있는 것 같습니다.
유지보수와 협업이 용이하다는 것은 변경에 유연한 것 같아요.
변경에 유연할려면 객체들이 각각이 당연히 있어야할 책임들이 갖고 있어야하겠죠.
지금은 너무 절차지향적인 코드인 것 같아요.
웬만하면 static을 다 해제하고 객체에게 상태를 부여해서 코드를 변경해보죠.
💡 리뷰 포인트
- 객체 책임 분리 (Ladder, Line, ... 래핑하기 )
- MVC 패턴
빠른 티키타가가 훨씬 좋을것 같아서. 짧게 리뷰 남기겠습니다.
앞으로는 빠르게 답변 드리겠으니, 리뷰하고 맨션 부탁드립니다.
궁금하신게 있으면 DM 주셔도됩니다!!
그리고 혹시 페어 프로그래밍을 같이 해보고 싶다면 요청하셔도 좋겠어요
| package controller; | ||
|
|
||
| import domain.LadderArray; | ||
| import domain.Participant; | ||
| import view.Input; | ||
| import view.Output; | ||
|
|
||
| import java.util.ArrayList; | ||
|
|
||
| public class Application { |
There was a problem hiding this comment.
[Comment]
java 프로그램에서 main 함수를 갖고 있는 클래스는 어플리케이션을 실행(run) 시키기 위해 많이 사용하는데요.
이 Application 클래스를 controller 패키지 밑에 위치 시킨데에 이유가 있을까요?
- Application
- 어플리케이션을 실행 시키기 위한 클래스
- controller
- view와 domain을 연결
저는 이 두가지로 이해하고 있는데요.
제 생각에도 Application을 저 두가지의 역할을 하도록 설계해도 전혀 문제될 것은 없다고 생각합니다.
하지만 지금 이 Application의 main 메서드가 너무 길어지고 view와 도메인을 오가서 프로그램의 flow를 한눈에 보기 쉽지 않은데요.
정리해보면 어떨까요?
| while(true){ | ||
| // 검색 | ||
| search(participants); | ||
| } |
There was a problem hiding this comment.
[Request Change]
break 조건이 없는 반복문은 예외가 터져야만 멈추겠네요 ㅠㅠㅠㅠ
| public static ArrayList<ArrayList<Integer>> createLadderArray(int x, int y) { | ||
| System.out.println(); | ||
| ArrayList<ArrayList<Integer>> array2D = new ArrayList<>(); |
There was a problem hiding this comment.
[Request Change]
위에서 리뷰 해주신 것처럼,
연결다리(Intger), 라인(List<Integer>), 사다리(List<List<Integer>>)가 각각 역할을 가질 수 있도록 클래스로 래핑해보면 어떨까요?
연결다리(Integer)
- 1일 때는 건널 수 있다.
- 0일 때는 건널 수 없다.
- 0과 1의 값만 가진다.
- 0과 1의 값중 랜덤하게 생성된다.
라인(List)
연결다리를 사람수만큼 갖는다.- 연속해서 건널 수 있는
연결다리는 가질 수 없다. - 참가자는 매 라인마다 건널 수있는
연결다리가 있으면 이동 / 없으면 이동 X
사다리(List<List>)
- 사다리 높이만큼
라인을 갖는다. - 높이는 2 이상이여야한된다.
- 참가자는 매 라인 이동이 끝나면 다음 라인으로 넘어간다.
이런 요구사항들을 도출해볼 수 있겠죠?
위의 예시는 제가 README를 작성한다면 뽑아내볼 요구사항을 간략히 적어본 것이고 저렇게 하라고 적은 것은 당연히 아닙니다.
아무래도 객체의 책임 분리에 대해서 어려움이 있으실 것 같아 예시를 남겨드린것이니,
준민님만의 요구사항을 도출해보고 객체를 나눠보는 것은 어떨까요?
- README를 통해 필요한 객체(모델, ...)의 역할 및 기능 나열
- README를 토대로 class 설계 및 작성
- 리팩터링하며, 책임분리 더 명확히 하기
이런 방식으로 연습해보면 좋을 것 같아요
| public class LadderArray { | ||
|
|
||
| // 가로 사다리 배열 만들기 1 | ||
| public static ArrayList<ArrayList<Integer>> createLadderArray(int x, int y) { |
There was a problem hiding this comment.
[Request Change]
준민님은 객체 지향 프로그래밍의 가장 큰 정체성은 무엇이라고 생각하시나요?
저는 객체가 상태(state)를 가진는 것이라고 생각해요!
LadderArray가 모두 static 메서드로 유틸 클래스로 선언되어있네요!!
유틸 클래스는 이곳 저곳에서 공용으로 사용할 수 있다는 특징이 있는데요.
LadderArray같은 도메인 모델이 상태를 갖도록 해보는 것은 어떨까요?
당장 왜 그래야되는지 상상이 어렵다면, 두가지 요구사항이 추가된다고 가정을 해보면 좋을 것 같아요
- 동시에 여러사람이 접속할 수 있는 게임이다. 여러ladderArray가 존재할 수 있다.
- 게임을 저장할 수 있다 이전에 사용한던 LadderArray를 불러와야 한다.
|
|
||
| // 가로 사다리 배열 만들기 1 | ||
| public static ArrayList<ArrayList<Integer>> createLadderArray(int x, int y) { | ||
| System.out.println(); |
There was a problem hiding this comment.
[Request Change]
현재 저희는 MVC 패턴을 이용해서 어플리케이션을 설계하고 있는데요.
MVC 패턴의 가장 큰 특징이라하면, 도메인 로직을 담당하는 Model과 유저와 맞닿아있는 View를 관리를 따로 하는 것인데요.
지금 LadderArray는 Model로서 작성된 코드로 보여지는데, View에 대한 코드가 있는 것 같아요.
지금은 View가 console이지만 웹 어플리케이션으로 변경된다는 View의 변경사항이 생기면 이 부분의 코드도 변경해야겠죠?
변경 사항이 전파된다는 것은 의존한다라고 볼 수 있는데요. 모델이 뷰를 의존하면 안될 것 같아요. 확실하게 나눠보죠!
| ArrayList<ArrayList<Integer>> array2D = new ArrayList<>(); | ||
|
|
||
| for (int i = 0; i < y; i++) { | ||
| array2D.add(generateRow(x)); |
There was a problem hiding this comment.
[RequestChange]
array2D와 같은 변수명은 코드를 짠 당시의 본인을 제외한 타인(미래의 본인, 팀원, 리뷰어)이 의도를 파악하기 힘들게 합니다.
도메인에 연관된 네이밍을 해주세요. (참고, 유비쿼터스 언어)
There was a problem hiding this comment.
controller 패키지에는 Controller 파일만 두고, Application 파일은 controller가 아닌 다른 위치에 두는게 좋을 것 같아요!(다른 분들 코드를 읽어보니, main 패키지를 하신분도 있었고 저는 패키지를 따로 두지 않고 java 패키지 밑에 두었습니다.
Application 클래스에는 Controller 클래스를 불러와서 게임을 진행하는 메서드를 실행시키면 될 것 같습니다. 저는 아래와 같이 작성했습니다:)
public class LadderApplication {
public static void main(String[] args){
LadderController controller = new LadderController();
controller.run();
}
}
| public static void main(String[] args) { | ||
|
|
||
| // 참여자 입력 | ||
| ArrayList<Participant> participants = Input.inputParticipants(); |
| // 반복 검색 | ||
| while(true){ | ||
| // 검색 | ||
| search(participants); | ||
| } |
There was a problem hiding this comment.
탈출 조건이 문제에 나와있지 않아 저도 적어야하나 많이 고민했는데요... 그래도 코드가 계속 돌아가는 문제를 막기 위해서는 while(true)문 탈출조건을 적어주는 것이 좋을 것 같습니다!
| if (previousValue == 1 && value == 1) { | ||
| value = 0; | ||
| } |
There was a problem hiding this comment.
previousValue가 두가지(0과 1)의 값만 가진다면, boolean형으로 바꿔도 될 것 같아요!
| ArrayList<Participant> participants = new ArrayList<>(); | ||
| for (String name : names) { | ||
| participants.add(new Participant(name)); | ||
| } |
There was a problem hiding this comment.
Input 클래스에서 Participant 객체를 생성하고 있네요!
View 계층에서 도메인 객체 생성 역할을 수행하지 않도록 다른 클래스로 옮기고, Input 클래스에서는 단순히 이름을 String 리스트로 반환하도록 수정하는건 어떨까요?
| int height = scanner.nextInt(); | ||
| scanner.nextLine(); |
There was a problem hiding this comment.
int height = Integer.parseInt(scanner.nextLine()); 로 작성하면 한줄로 줄일 수 있습니다!
| if(val == 1) | ||
| System.out.print("-----"); | ||
| if(val == 0) | ||
| System.out.print(" "); |
| } | ||
|
|
||
| // 개별 참가자의 결과 출력 2 | ||
| public static void printResultForParticipant2 (Participant p, String name){ |
There was a problem hiding this comment.
위에서 말씀드렸던 것 처럼, p라는 변수보다 더 명확한 변수명을 사용하시는걸 추천드려요!
| public class LadderArray { | ||
|
|
||
| // 가로 사다리 배열 만들기 1 | ||
| public static ArrayList<ArrayList<Integer>> createLadderArray(int x, int y) { |
There was a problem hiding this comment.
기본 프로그래밍 요구사항에 "줄여 쓰지 않는다(축약 금지)" 라는 부분이 이 있는데요 명확한 변수를 사용하면, 코드의 가독성을 높이고, 협업시 혼란을 주는 것을 방지한다고 하네요!
x와 y라는 변수명도 좋지만, 조금 더 명확한 변수명을 사용하시는걸 추천드려요!
controller - Application
domain - Participant, LadderArray
view - Input, Output
테스트 아이디어가 생각나지 않아서 테스트는 조금 더 고민해보고 작성해보겠습니다 :)
<실행 결과>
참여할 사람 이름을 입력하세요. (이름은 쉼표(,)로 구분하세요)
neo,brown,brie,tommy
실행 결과를 입력하세요. (결과는 쉼표(,)로 구분하세요)
1000,2000,3000,4000
최대 사다리 높이는 몇 개인가요?
5
사다리 결과
결과를 보고 싶은 사람은?
all
실행 결과
neo : 2000
brown : 4000
brie : 3000
tommy : 1000
결과를 보고 싶은 사람은?
neo
실행 결과
2000
결과를 보고 싶은 사람은?
brown
실행 결과
4000
결과를 보고 싶은 사람은?
brie
실행 결과
3000
결과를 보고 싶은 사람은?
tommy
실행 결과
1000