[또링] 체스 4~5단계 미션 제출합니다. #154

jnsorn · 2020-04-08T11:25:48Z

안녕하세요, 구구님! 1~3단계 마무리 하면서 주셨던 코멘트들 수정 완료하였습니다 😄
꼼꼼히 봐주셔서 정말 감사합니다. 코멘트 관련해서 질문이 있는데, 여기가 더 보기 편하실 것 같아 질문 몇 개 남깁니다!

board에서 역순서로 반환하는 기능을 만들라고 하셨고, 그건 View의 기능으로 보기보다는 Rank 리스트를 역순으로 반환하는 기능 추가라고 생각하라고 하셨는데요,
그렇게 되면 Board에서 출력을 위해 piece가 없는 column에 .을 갖고 있게 하는 로직 도 그렇게 생각할 수 있을까요? 저만의 기준이 잡히지 않는 것 같아 리뷰어님의 생각을 여쭤봅니다..!😅
기존 Rank와 Board내에 계산하는 로직이 섞여있었고, 해당 기능들을 score로 분리하였는데요! 이 과정에서 board내의 rank를 알아야하고 rank내의 존재하는 piece들을 알아야하는 상황이 발생하여 get을 이용하게 되었습니다..😥 구구님이 코멘트 달아주신대로 rank가 점수를 계산하는 함수를 갖고 있다면 getter를 사용하지 않고 점수를 구할 수 있는데요, 이렇게 된다면 다시 계산을 하는 로직이 여기 저기로 분산된다고 생각합니다..! 이 때, 어떻게 구현해야 더욱 객체지향적으로 짤 수 있는지 고민이 되네요.

getter/setter/property를 쓰지 않는다.
만약 객체가 지금 인스턴스 변수의 적당한 집합을 캡슐화하고 있지만 그 설계가 여전히 어색하다면, 좀 더 직접적인 캡슐화 위반을 조사해볼 때다. 그냥 단순히 현재 위치에서의 값을 물을 수 있는 동작이라면 해당 인스턴스 변수를 제대로 따라가지 못할 것이다. 강한 캡슐화 경계의 바탕에 깔린 사상은 동작의 검색과 배치를 위해 남겨둔 코드를 만질 다른 프로그래머를 위해 객체 모델의 단일한 지점으로 유도하려는 것이다. 이는 많은 긍정적인 하부효과를 가져다 주는데, 중복 오류의 극적 축소와 새 기능의 구현을 위한 변경의 지역화 개선 등이 있다. 이 규칙은 흔히 “말을 해, 묻지 말고”라고 일컫는다.

이러지도 저러지도 못하는 상황이 발생하게 된 건 설계가 잘못된 것일까요?... 구구님의 생각이 궁금해서 질문 남깁니다..!

kang-hyungu · 2020-04-09T23:17:43Z

Board 클래스 내에 어느 부분을 말하는걸까요? getRanks 메서드 말고 출력을 위한 기능이 안보여서 질문을 잘 파악 못했어요;
BoardConverter 부분 말하는걸까요?
Score, Rank 클래스로 계산 로직이 각각 있는 것도 문제네요. board에서 getRank해서 Score에 넘기지 말고 board에서 아예 piece 리스트를 전달하면 계산 로직을 score로 모을 수 있겠네요

kang-hyungu

또링 코드 깔끔하게 잘 작성하셨어요
board 데이터를 가져와서 처리하기 위한 부분을 조금 다듬어보면 좋을 것 같네요
피드백 반영해보시고 궁금한 점 생기면 코멘트 남기거나 dm 주세요

src/main/java/domain/board/Rank.java

src/main/java/domain/piece/team/Team.java

src/main/java/web/dao/BoardDao.java

src/main/java/web/service/BoardService.java

src/main/resources/templates/game.html

src/main/java/web/util/BoardConverter.java

src/main/java/web/dto/ChessGameDto.java

jnsorn · 2020-04-12T06:43:55Z

1. Board 클래스 내에 어느 부분을 말하는걸까요? getRanks 메서드 말고 출력을 위한 기능이 안보여서 질문을 잘 파악 못했어요; BoardConverter 부분 말하는걸까요?

제가 너무 질문을 이상하게 한 것 같네요..!
OutputView에 아래와 같이 빈 부분을 EMPTY_CELL_SYMBOL(.) 으로 채워서 출력해주는 기능이 있는데요,

	private static void printRank(List<Piece> rank) {
		for (int i = MIN_COLUMN_COUNT; i <= MAX_COLUMN_COUNT; i++) {
			final int columnNumber = i;
			String pieceSymbol = rank.stream()
				.filter(p -> p.equalsColumn(columnNumber))
				.map(Piece::showSymbol)
				.findFirst()
				.orElse(EMPTY_CELL_SYMBOL);
			System.out.print(pieceSymbol);
		}
		System.out.println();
	}

이런 함수도 OutputView에서 처리하는 게 아닌, Rank의 한 가지 기능(빈 부분을 empty symbol로 채워주어 리턴하는 기능)이라고 보고, Rank에 해당 메서드를 추가할 수 있을지가 궁금했습니다.
(다만 rank에 이런 함수가 추가된다면 System.out.print() 대신 add()가 되어 List<Piece>를 리턴하도록 할 수 있겠네요!)

여기부터는 위와 관련된 저의 생각들입니다..!

제가 이전에 드렸던 질문입니다!

board에서 역순서로 반환하는 기능을 만들라고 하셨고, 그건 View의 기능으로 보기보다는 Rank 리스트를 역순으로 반환하는 기능 추가라고 생각하라고 하셨는데요,

그렇게 되면 Board에서 출력을 위해 piece가 없는 column에 .을 갖고 있게 하는 로직 도 그렇게 생각할 수 있을까요? 저만의 기준이 잡히지 않는 것 같아 리뷰어님의 생각을 여쭤봅니다..!

-> 사실 구구님이 처음에 제안해주신 부분(위의 첫 문장)은 다른 크루들과 이야기를 나누며 지금은 조금 정리가 되었습니다.
제가 내린 결론은 reverse와 같은 로직은 사실상 이름대로 뒤집혔다기 보다는 원래의 보드 상태를 의미하기 때문에 domain이 해당 기능을 갖고 있어도 이상하지 않다입니다!

-> 하지만 제가 제시한 경우(위의 두번째 문장)에도 마찬가지로 적용될 수 있을지는 아직 생각이 정리되지 않았습니다.

처음 코드를 구현할 때에는 아래와 같은 이유로 OutputView에 해당 메서드를 넣어주었습니다.

출력을 위해 빈 부분을 채워주는 것이지, 실제 도메인내에서 로직들을 수행하는데에는 필요하지 않기 때문에
empty symbol을 다르게 표시하고 싶다는 요구사항이 들어오면 domain이 수정될 가능성이 있어서..? -> 이 부분은 domain이 view의 emptySymbol 상수를 사용하면 해결되는 문제네요^^;

하지만 웹이 추가되고 나서 빈 자리를 알아야하는 상황이 발생하였고, OutputView의 printRank가 거의 비슷하게 재작성되는 일이 발생했습니다.
(BoardConverter의 List<String> toSymbolsFromBoard(List<Rank> board))

이런 상황에서, 여러가지 뷰/클라이언트에서 해당 기능이 필요하다는 것은 곧 domain이 갖고있어도 마땅한 기능이라고 생각하면 되는 걸까요?

물론 여러 프로그램을 구현해보면서 저만의 생각을 정리해나가야겠지만 조금 혼란스러워서 드렸던 질문입니다..!😊

2. Score, Rank 클래스로 계산 로직이 각각 있는 것도 문제네요. board에서 getRank해서 Score에 넘기지 말고 board에서 아예 piece 리스트를 전달하면 계산 로직을 score로 모을 수 있겠네요

getter와 객체 책임에 대한 고민을 모두 해결해줄 수 있는 좋은 방법인 것 같습니다!
board에서 piece를 반환하여 score는 해당 책임을 담당할 수 있도록 수정 완료하였습니다.

아무래도 글로 제 복잡한 생각들을 표현하다보니 지저분해졌습니다..!😥
천천히 읽고 답변 주시면 더 고민해보겠습니다😄

kang-hyungu · 2020-04-15T22:07:38Z

Q. 여러가지 뷰/클라이언트에서 해당 기능이 필요하다는 것은 곧 domain이 갖고있어도 마땅한 기능이라고 생각하면 되는 걸까요?
A. 뷰에 비슷한 로직이 나온다고 도메인에서 처리해야될 기능이라고 보긴 어렵습니다. 빈칸을 어떻게 화면에 표현할지 결정은 뷰에서 하는 것이니 도메인은 뷰에서 잘 처리할 수 있도록 약속된 값만 반환하면 됩니다.
콘솔, 웹 둘다 "."으로 표현하고 있는거죠? 관심사가 다른 뷰이니 비슷한 로직이라도 각자 구현해주는게 맞습니다.

kang-hyungu

또링, 피드백 반영 잘 하셨어요 👏
몇 가지 간단한 피드백 남겼어요
확인해보시고 궁금한 점 생기면 코멘트 남기거나 dm 주세요!

kang-hyungu · 2020-04-15T22:35:08Z

src/main/java/web/service/PieceService.java

+import web.util.PieceFactory;
+import web.util.ScoreConverter;
+
+public class PieceService {


PieceService인가요?
보드게임과 관련된 로직들로 보이는데 클래스명이 적절해보이진 않네요

초기에 piece와 관련된 기능을 수행할 용도로 클래스 명을 pieceService로 지었는데, 중간에 turn 등이 추가되면서 게임과 관련된 클래스가 되었습니다..! 해당 로직에 맞게 클래스명 변경하였습니다😄

kang-hyungu · 2020-04-15T22:38:42Z

src/main/java/web/util/PieceFactory.java

+			.orElseThrow(() -> new IllegalArgumentException("해당 말이 존재하지 않습니다."));
+	}
+
+	public static String convert(String symbol) {


팩토리 클래스에서 값을 변환하는 기능도 제공할 필요는 없어보이네요
클래스명처럼 Piece를 생성하는 책임만 가져야 하지 않을까요?

다른 코멘트 수정하며 이 부분도 함께 해결되었습니다! 감사합니다☺

kang-hyungu · 2020-04-15T22:41:53Z

src/main/java/web/util/PieceFactory.java

+import domain.piece.team.Team;
+
+public enum PieceFactory {
+	WHITE_PAWN("p", "♙", position -> new Pawn(Position.of(position), Team.WHITE)),


unicode는 원래 뷰에서 처리할 값으로 보이는데 자바스크립트를 따로 활용하지 않고 백단에서 처리하려다 보니 나온 값으로 보이네요

자바스크립트를 잘 사용할줄 몰라서 최대한 백단에서 처리하려했습니다..! 우선은 UnicodeConverter로 분리하였는데, 아무래도 js를 활용해서 앞에서 보여줄 때 변환하는 것이 맞을까요?

보통은 프론트에서 처리하는게 좋다고 생각합니다.
UnicodeConverter 같이 뷰 레이어에서 처리하도록 분리하는 방법도 괜찮네요

kang-hyungu · 2020-04-15T22:43:59Z

src/main/java/web/util/PieceFactory.java

+import domain.piece.position.Position;
+import domain.piece.team.Team;
+
+public enum PieceFactory {


팩토리가 util 패키지로 묶이는게 이상하네요
유니코드 처리와 dao에 저장할 객체를 생성하는 용도로 쓰면서 이것저것 혼합된 클래스에요
이 클래스가 필요한지 잘 모르겠네요

유니코드 처리는 UnicodeConverter를 따로 생성하여 역할을 분리하였습니다!
추가로 이 클래스는 symbol과 position을 받아 piece로 변환해주는 역할을 하기 때문에 PieceConverter로 이름 수정하였습니다..!😅😊

kang-hyungu · 2020-04-15T22:49:19Z

src/main/java/web/dao/PieceDao.java

+	private static final PieceDao PIECE_DAO = new PieceDao();
+	private static final String TABLE_NAME = "piece";
+
+	private DBConnector connector = new DBConnector();


DBConnector를 싱글톤으로 만들면 어떨까요
dao마다 다른 DBConnector를 유지할 필요가 없어 보여요
getConnection를 static 메서드로 만들거나요

좋은 지적 감사합니다~ getConnection을 static으로 선언하여 해당 문제 해결하였습니다😀

kang-hyungu · 2020-04-15T22:53:03Z

src/main/java/web/dao/PieceDao.java

+	public void add(PieceDto pieceDto) throws SQLException {
+		String query = "INSERT INTO " + TABLE_NAME + " VALUES(?,?,?)";
+		try (
+			PreparedStatement pstmt = connector.getConnection().prepareStatement(query)


Suggested change

PreparedStatement pstmt = connector.getConnection().prepareStatement(query)

Connection conn = connector.getConnection();

PreparedStatement pstmt = conn.prepareStatement(query)

지금처럼 작성하면 PreparedStatement만 close 동작하지 않을까요?

좋은 지적 감사합니다😀
connection의 close도 동작시키기 위해서 따로 선언을 한 후 사용을 해줘야겠네요!
try with resources 문을 처음 사용해봐서 이런 부분을 놓친 것 같아요..😂
관련된 부분들 모두 수정하였습니다.

kang-hyungu · 2020-04-15T22:55:56Z

src/main/java/web/dao/PieceDao.java

+		) {
+			rs = pstmt.executeQuery();
+			while (rs.next()) {
+				PieceFactory piece = PieceFactory.of(rs.getString("symbol"));


보통 PieceFactory로 객체를 생성하면 Piece가 반환될 것을 기대하는데 PieceFactory가 반환되네요
Piece를 바로 생성하도록 바꿔볼까요?

symbol과 position을 함께 전달하여 해당 piece가 바로 반환되도록 수정 완료하였습니다! 어쩐지 어색하다고 느껴졌던 부분인데 코멘트덕에 잘 해결되었습니다👍👍👍

kang-hyungu · 2020-04-15T22:57:05Z

src/main/java/web/dao/TurnDao.java

+		) {
+			rs = pstmt.executeQuery();
+			if (!rs.next())
+				return null;


null 반환하지 않는게 좋습니다
NPE 발생 시키는 원인이 됩니다

좋은 지적 감사합니다! rs가 존재하지 않을 때에는 TurnDto에 빈 문자열을 넣어주어 예외를 발생시키도록 수정하였습니다 😊

kang-hyungu · 2020-04-15T22:59:17Z

src/main/java/web/service/PieceService.java

+		String originalPosition = moveCommand.getSourcePosition().toString();
+		String newPosition = moveCommand.getTargetPosition().toString();
+		updateTurn(boardGame);
+		return updateBoard(boardGame, originalPosition, newPosition);


updateBoard 메서드가 두 가지 일을 하네요
위치 정보 저장하는 일
dto로 변환하는 일
메서드는 한 가지 일만 하는게 좋습니다

Dto로 변환하는 메서드를 작성하여 해당 문제 해결하였습니다! 감사합니다😀😊

kang-hyungu · 2020-04-15T23:00:19Z

src/main/java/web/service/PieceService.java

+		return new BoardDto(symbols, ScoreConverter.convert(score));
+	}
+
+	private List<String> convert(List<Rank> board) {


ScoreConverter처럼 컨버터를 따로 두는게 낫겠네요
2뎁스는 1뎁스로 줄일 필요도 있겠네요

원래 UnicodeConverter로 관리했다가, 각각의 piece에 따른 unicode라고 생각해서 다시 합쳤는데 역시 따로 두는 게 더욱 깔끔해보이네요! depth도 메소드를 분리하여 수정완료하였습니다!😀

jnsorn · 2020-04-17T08:55:40Z

구구님! 코멘트 달아주신부분 수정 완료하여 리뷰요청 드렸습니다!
한 가지 궁금한 게 있습니다.

web으로 프로그램을 구현할 때 테스트는 어느 부분까지 이루어져야하는지 모르겠습니다!

console로 프로그램 작성 시, 주로 도메인을 테스트 하였는데 web은 dao service controller 중 어느정도까지 테스트가 되는게 보편적인가요? dao와 service 테스트를 하면 되는건지... 현업에서는 어떻게 테스트를 하시는지 궁금해서 여쭤봅니다!😊 (지금 web 프로그램을 작성하면서 테스트코드를 많이 못짰는데, 답변주시면 테스트코드도 더 보완해보겠습니다😂!)

kang-hyungu · 2020-04-19T00:24:35Z

레벨1 과정에서는 도메인 테스트로 충분하다고 생각합니다.
다음 과정에서 스프링 배울 때 인수 테스트도 작성해볼거에요

kang-hyungu

또링 피드백 반영 잘 하셨어요 👏
질문 주신 내용은 다음 과정 진행하면서 해결될거에요
미리 공부해보고 싶으시면 최근 최범균님이 새로 출판하신 tdd 책 참고하세요
https://book.naver.com/bookdb/book_detail.nhn?bid=16267566
pr은 머지 처리할게요
미션 진행하느라 수고하셨어요!

jnsorn added 15 commits April 8, 2020 15:09

refactor : toMap을 활용하여 점수 계산 로직 변경

e1c88f0

refactor : pawn의 상태를 삭제하고 처음 2칸 움직임을 해당 폰의 row로 판단하도록 수정

4f913e5

refactor : hasPieceInRoute메서드 인덴트 수정

5301967

refactor : score에서 getter를 사용하지 않도록 rank에 메서드 추가

79f257f

refactor : 같은 column에 위치한 pawn이 있는지 확인하는 메서드 수정

1bea3f8

docs : 4~5단계 구현 기능 목록 작성

158369c

feat : 게임을 진행할 html 구현

b04979d

refactor : 메서드 순서 변경 및 사용하지 않는 메서드 제거

a92045c

refactor : game.html 변경

96c6aee

feat : controller 구현

3cf4a09

feat : 말을 unicode로 바꿔주는 정적클래스 구현

ac0fa0c

feat : DTO 생성

11a37c4

feat : DAO 셍성 및 테스트 코드 작성

dad7c67

feat : service 생성

f1a0dfe

feat : load하는 기능 구현

20aad54

kang-hyungu requested changes Apr 9, 2020

View reviewed changes

jnsorn added 3 commits April 12, 2020 14:38

refactor : 복잡한 메서드 명 변경

3165c41

refactor : convention 위배한 부분 수정

9249a5e

refactor : rank로 부터 score 기능 분리

3b0bdfb

jnsorn added 7 commits April 13, 2020 01:30

refactor : DAO에서 DB 연결 부분 분리

4c66e0f

refactor : 테이블 재설계 및 게임 저장 기능 구현

02faafb

refactor : 게임 불러오는 기능 수정

a8266f3

docs : README 수정

7312608

refactor : ResponseDto를 BoardDto로 수정

e12eed9

refactor : 점수가 보이도록 수정

eee4950

refactor : Unicode로 변환해주는 책임을 PieceFactory가 갖도록 수정

a629f7b

kang-hyungu requested changes Apr 15, 2020

View reviewed changes

jnsorn added 8 commits April 17, 2020 15:04

refactor : 적절하지 않은 클래스명 변경

be73503

refactor : connection 자원도 관리가 되도록 try-with-resources문 수정

bfdd40e

refactor : DBConnector를 static으로 변경

308e64d

refactor : null을 리턴하던 부분을 수정하여 npe에 방어적으로 작성

0b1bcf6

refactor : piece factory에서 unicode 변환해주는 책임을 분리

bc2f21e

refactor : service에서 Dto를 생성하는 로직을 메서드로 추출

4d3675a

refactor : piecefactory에 symbol과 position을 전달하면 해당 piece반환

5a08889

refactor : PieceFactory->PieceConverter로 이름 수정

c331ea3

kang-hyungu approved these changes Apr 19, 2020

View reviewed changes

kang-hyungu merged commit b7fde61 into woowacourse:jnsorn Apr 19, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[또링] 체스 4~5단계 미션 제출합니다. #154

[또링] 체스 4~5단계 미션 제출합니다. #154

jnsorn commented Apr 8, 2020 •

edited

Loading

kang-hyungu commented Apr 9, 2020

kang-hyungu left a comment

jnsorn commented Apr 12, 2020 •

edited

Loading

kang-hyungu commented Apr 15, 2020

kang-hyungu left a comment

kang-hyungu Apr 15, 2020

jnsorn Apr 17, 2020

kang-hyungu Apr 15, 2020

jnsorn Apr 17, 2020

kang-hyungu Apr 15, 2020

jnsorn Apr 17, 2020

kang-hyungu Apr 19, 2020

kang-hyungu Apr 15, 2020

jnsorn Apr 17, 2020

kang-hyungu Apr 15, 2020

jnsorn Apr 17, 2020

kang-hyungu Apr 15, 2020

jnsorn Apr 17, 2020

kang-hyungu Apr 15, 2020

jnsorn Apr 17, 2020

kang-hyungu Apr 15, 2020

jnsorn Apr 17, 2020

kang-hyungu Apr 15, 2020

jnsorn Apr 17, 2020

kang-hyungu Apr 15, 2020

jnsorn Apr 17, 2020

jnsorn commented Apr 17, 2020 •

edited

Loading

kang-hyungu commented Apr 19, 2020

kang-hyungu left a comment

	PreparedStatement pstmt = connector.getConnection().prepareStatement(query)
	Connection conn = connector.getConnection();
	PreparedStatement pstmt = conn.prepareStatement(query)

[또링] 체스 4~5단계 미션 제출합니다. #154

[또링] 체스 4~5단계 미션 제출합니다. #154

Conversation

jnsorn commented Apr 8, 2020 • edited Loading

kang-hyungu commented Apr 9, 2020

kang-hyungu left a comment

Choose a reason for hiding this comment

jnsorn commented Apr 12, 2020 • edited Loading

1. Board 클래스 내에 어느 부분을 말하는걸까요? getRanks 메서드 말고 출력을 위한 기능이 안보여서 질문을 잘 파악 못했어요; BoardConverter 부분 말하는걸까요?

2. Score, Rank 클래스로 계산 로직이 각각 있는 것도 문제네요. board에서 getRank해서 Score에 넘기지 말고 board에서 아예 piece 리스트를 전달하면 계산 로직을 score로 모을 수 있겠네요

kang-hyungu commented Apr 15, 2020

kang-hyungu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jnsorn commented Apr 17, 2020 • edited Loading

kang-hyungu commented Apr 19, 2020

kang-hyungu left a comment

Choose a reason for hiding this comment

jnsorn commented Apr 8, 2020 •

edited

Loading

jnsorn commented Apr 12, 2020 •

edited

Loading

jnsorn commented Apr 17, 2020 •

edited

Loading