[Starve] Mission5 - 보드에 위치 부여 및 점수 계산 #148

Jiwon-JJW · 2021-02-24T06:24:45Z

안녕하세요! 미션 5를 완료 하여, 다섯번째 PR 요청 드립니다.

변경 사항

Piece

중복코드를 최소화 하기 위하여, createWhite()와 createBlack() 메서드를 생성하였습니다. 본래 있던 create 메서드들을 통합하여 Type과 Position을 받을까 고민을 했지만, 타입과 색상을 강제하는 것이 더 안전할 것 같아, 그대로 체스말의 종류와 색상을 강제하도록 진행하였습니다.
Piece에 Position을 갖도록 하여 체스 말 생성 시와, 이동할 경우, 위치를 설정할 수 있는 메서드들을 추가하였습니다.
체스말 별 Point를 얻을 수 있는, getPoint 메서드를 생성하였습니다.
만약, pawn이 놓여있는 세로줄에 같은 pawn이 있을 경우, 점수를 반만 얻을 수 있도록 설정 하였습니다.
포인트 순으로 내림차순 정렬하기 위하여 Comparable 인터페이스를 implements 하여, 메서드를 생성하였습니다.

Type

Point를 관리하는 double 변수를 생성하였습니다.

Board

Piece 리스트를 관리하는 Rank를 담는 ranks 배열을 생성하였습니다.
색상과 타입별로 개수를 세어주는 메서드와, 색상별 모든 체스말의 개수를 세어주는 메서드를 생성하여, 총 체스말의 개수를 세는 메서드를 생성하였습니다.
체스말을 움직일 수 있는 move 메서드를 추가하였습니다.
현재 체스판에 있는 체스말들의 포인트를 계산하는 메서드를 추가하였습니다.

Position

체스말의 위치를 관리하는 클래스를 생성하였습니다. a8같은 형식으로 입력되는 체스말의 위치를 숫자로 변환할 수 있습니다.

Rank

체스말을 관리하는 Rank 클래스를 생성하였습니다. 체스말을 초기화 하거나, 체스말의 위치를 변경할 때 사용할 수 있는 메서드들을 입력하였습니다.

여기까지 읽어주셔서 감사합니다!

…스말 과 공백 추가하는 add

… 얻을 수 있는 메서드 추가

LarryJung

안녕하세요~ 코드 깔끔하게 잘 구현하셨네요!
크게 어려움 없이 리뷰 할 수 있었구요ㅎㅎ

좀 더 나은 코드 패턴과 설계를 위해 의견을 좀 드렸으니 확인 부탁드립니다.~

LarryJung · 2021-02-24T16:12:34Z

src/main/java/chess/Board.java

+    public int totalPiecesCount() {
+        return countingAllPiecesByColor(Color.WHITE) + countingAllPiecesByColor(Color.BLACK);


Rank가 좀 더 많은 일을 할 수 있을 것 같은데요,

return ranks.stream().mapToInt(rank -> rank.getPieceCount()).sum();

과 같은 형식으로 바꾼다면 어떻게 하면 좋을까요??

LarryJung · 2021-02-24T16:14:11Z

src/main/java/chess/Board.java

+    public int countingAllPiecesByColor(Color color){
+        int count = 0;
+        count += countPiecesByColorAndType(color, Type.PAWN);
+        count += countPiecesByColorAndType(color, Type.ROOK);
+        count += countPiecesByColorAndType(color, Type.KNIGHT);
+        count += countPiecesByColorAndType(color, Type.BISHOP);
+        count += countPiecesByColorAndType(color, Type.QUEEN);
+        count += countPiecesByColorAndType(color, Type.KING);
+        return count;


enum의 values 메서드를 알고 계신가요?

Type.values() 를 활용한 방식으로 바꿔보면 좋을 것 같습니다.

LarryJung · 2021-02-24T16:16:53Z

src/main/java/chess/Board.java

+        int countPiece = 0;
+        for (Rank rank : ranks) {
+            countPiece += rank.getCountPiecesByColorAndType(color, type);
+        }


위에
return ranks.stream().mapToInt(rank -> rank.getPieceCount()).sum();

예시를 드린 것으로 리팩토링을 진행해 보시면 좋을 것 같습니다~

이미 스트림을 조금 활용하고 계신 분들도 보이는데요, 포문도 좋지만 한번 간단하게 학습해보시는것은 어떨까요?ㅎㅎ

LarryJung · 2021-02-24T16:28:16Z

src/main/java/chess/Board.java

+    public List<Piece> findPiecesByColor(Color color) {
+        List<Piece> pieces = new ArrayList<>();
+
+        for (Rank rank : ranks) {
+            pieces.addAll(rank.findPieceByColor(color));
        }
+
+        return pieces;


전체적으로 리스트 순회하는 부분이 코드가 비슷하네요ㅎㅎ

데이터 선언

포문

포문안에서 데이터 조작

깔끔하고 나무랄데 없는 코드라고 생각이 들지만
로직이 많이 복잡해졌을 때 스트림이 포문보다 가독성이 더 좋아진다고 생각합니다. (사실 의견은 분분합니다.^^)

개인적인 선호로는 스트림은 연산 결과가 무조건 값으로 반환되기 때문에 좀 더 선언적인 코드를 짤 수 있어서 좋다고 생각합니다.^^
물론 단점도 있어서 마구 남용하면 안되지만요~

포문과 스트림을 비교한 글이 있어서 링크드립니다.
https://blog.jdriven.com/2019/10/loop/

주의점은 스트림은 포문의 다른 형태로 나타난 것은 절대 아니니, 포문vs스트림 으로 비교한 것은 리스트 순회 로직의 측면에서만 비교했다는 것으로 생각해주시면 좋을 것 같네요~

저도 스트림을 써서 보기 좋을 때가 있고 아닐 때가 있더라구요. 그리고 스트림으로만 쉽게 되고 for문은 어려운 경우도 있기도 하구요. 연습 때는 이것 저것 맘에 드는 걸로 해 보면 좋을 것 같아요.
회사 아재들은 스트림을 싫어하는 분들이 많았는데 요즘은 어떨런지 모르겠네요 ㅎㅎ

LarryJung · 2021-02-24T16:29:06Z

src/main/java/chess/Board.java

    }

-    private String getPiecesSort(Color color) {
+    public Piece findPiece(String position) {
+        Position chessBoardIndex = new Position(position);


변수 이름을 position이라고 하면 어색한가요?

이름이 중복되어 position으로 지정하지 않았던 것이었습니다! position을 조금 변경해서 변수명을 수정해봐야겠습니다! 감사합니다~

LarryJung · 2021-02-24T16:37:09Z

src/main/java/chess/Rank.java

+    public static Rank initializeBlackPawns(int fileIndex) {
+        Rank rank = new Rank();
+        for (int rankIndex = 0; rankIndex < Board.BOARD_SIZE; rankIndex++) {
+            rank.addBlackPieces(createBlackPawn(new Position(rankIndex, fileIndex)));
+        }
+        return rank;
+    }
+
+    public static Rank initializeBlank(int fileIndex) {
+        Rank rank = new Rank();
+        for (int rankIndex = 0; rankIndex < Board.BOARD_SIZE; rankIndex++) {
+            rank.addBlank(createBlank(new Position(rankIndex, fileIndex)));
+        }
+        return rank;
+    }


이렇게 생긴 메서드가 조금 중복처럼 보이는데, 리팩토링 한번 해보실 수 있을까요?

넵 시도해보겠습니다!

LarryJung · 2021-02-24T16:38:21Z

src/main/java/chess/Rank.java

+    public int getCountPiecesByColorAndType(Color color, Type type) {
+        int countPiece = 0;
+
+        for (Piece piece : pieces) {
+            if (piece.matchColorAndType(color, type)) {
+                countPiece += 1;
+            }
+        }
+
+        return countPiece;
+    }
+
+    public List<Piece> findPieceByColor(Color color) {
+        List<Piece> pieceByColor = new ArrayList<>(Board.BOARD_SIZE);
+
+        for (Piece piece : pieces) {
+            if (piece.matchColor(color)) {
+                pieceByColor.add(piece);
+            }
+        }
+
+        return pieceByColor;
+    }


이런 느낌도.. 같은듯 다른듯.. 비슷한듯 아닌듯 한데....

이 또한 중복을 해결할 수 있다고 한다면 어떻게 할 수 있을까요?ㅎㅎㅎ 살짝 도전과제 느낌으로 질문 드립니다!

LarryJung · 2021-02-24T16:42:15Z

src/main/java/chess/Board.java

-        addBlackPieces(Piece.createBlackKnight());
-        addBlackPieces(Piece.createBlackRook());
+        ranks.get(chessBoardIndex.getFile()).move(chessBoardIndex.getRank(), piece);
+        piece.setPosition(chessBoardIndex);


piece.setPosition(chessBoardIndex) 는 rank의 move() 메서드 내부로 이동시키면?
캡슐화 좀 더 고려한 설계라고 생각이 듭니다. rank가 스스로 가지고 있는 piece를 책임지는 느낌도 들구요~

LarryJung · 2021-02-24T16:44:47Z

src/main/java/chess/pieces/Piece.java

+    @Override
+    public String toString() {
+        return String.valueOf(getRepresentation());
+    }
+
+    @Override
+    public int compareTo(Piece o) {
+        return Double.compare(getDefaultPoint(), o.getDefaultPoint()) * -1;
+    }


이런 종류의 자바 기본 인터페이스 메서드는 클래스 가장 아래에 두는게 일반적입니다.
(toString, equals, hashCode, compareTo ...)

LarryJung · 2021-02-24T16:45:25Z

src/main/java/chess/pieces/Piece.java

+        double halfPawnPoint = 0.5;
+        List<Position> positions = this.position.getFileNeighborPosition();
+        for (Position position : positions) {
+            if (pieces.contains(new Piece(this.color, this.type, position))) {


요거 piece를 새로 생성해도 로직에 문제가 없나요?

리스트 같은 곳에 따로 넣는 것이 아니어서 문제가 없다고 생각했습니다..! 그저 같은 Piece가 있는지 확인하는 용도로 생성하는 것이어서요..!

Jiwon-JJW added 30 commits February 22, 2021 17:06

체스말이 Blank일 때, '.'이 생성되는지 확인하는 코드 추가

441d96b

체스판의 빈 칸에 '.'을 생성할 수 있도록 createBlank 메서드 추가

0132e1a

NO_COLOR 항목 추가. BLANK 일 때 사용

3ac08a1

BLANK 항목 추가. BLANK 일 때 사용

43153c3

중복 메서드 제거를 위해 색상별 create 메서드 생성

a357c1c

타입과 컬러를 대조하여 비교해보는 메서드 추가

8a7c7f0

printBoard 메서드 수정 - 띄어쓰기 고정 및 rank와 file의 인덱스 추가

05d5a5f

특정 인덱스의 체스말을 찾는 finePiece 추가

83caa7b

Rank 클래스 생성

7a3f85f

Piece를 담는 pieces 리스트 생성

d27247a

기본적인 get메서드와 add 메서드 추가. - 리스트를 반환하는 getPieces, 체스말을 반환하는 getPiece, 체…

3c9c07b

…스말 과 공백 추가하는 add

기본세팅을 담당하는 initialize 메서드들 추가

837f717

컬러와 타입에 따라 체스말을 확인하여 개수를 세는 메서드 추가

28c4525

색상별로 생성되어있던 pieces List를 수정하여 Pieces를 담는 Rank를 갖는 files 리스트 생성

11ecf6e

색상과 타입 별로 체스말의 개수를 세는 메서드를 추가하여, total 개수를 세는 메서드도 수정

996a4d4

특정 위치에 있는 체스말을 찾는 findPiecd 메서드 추가

a718924

rank를 읽어, String으로 출력하는 메서드 추가

99c9b1d

files에 rank라인들을 넣도록 초기화 메서드 수정

4ba0982

메서드들의 변경으로 인하여 수정 작업 진행

8352917

비어있는 체스판을 생성하는 메서드 생성

dd2ffdb

move 메서드의 테스트를 위한 메서드를 생성

6cc1adf

체스말을 옮기는 메서드 생성

f655d1c

List에서 체스말을 set해주는 메서드 생성

cab6800

색상을 확인하는 matchColor 메서드 생성

391dd45

Type enum에 point 추가

0cf7cf0

Point 점수의 합계를 구하는 테스트 메서드 구현

dbc227a

색상을 기준으로 체스말을 찾는 메서드 구현

8383330

색상 별로 점수를 합쳐 반환하는 메서드 구현

e665d14

체스말의 위치를 얻을 수 있는 Position 클래스 추가로 인한 create에 position인자 추가와, x와 y의 값을…

6ead935

… 얻을 수 있는 메서드 추가

체스말의 위치를 얻을 수 있는 Point 클래스 추가하여 기본 메서드 작성

3f5eda2

Jiwon-JJW added 19 commits February 23, 2021 16:24

Position 추가로인한 메서드 수정

3cd3a2d

Position 추가로인한 메서드 수정

c4ded83

Position 추가로인한 메서드 수정

e87e443

assertAll로 코드 수정

a7c2c74

세로열 기준으로 인접한 체스말의 위치를 리스트 배열에 담아 반환하는 메서드 생성

234625c

코드 정렬작업 진행

9ba4457

총 포인트 합계를 구하는 테스트 메서드 작성 완료

4cfce5d

총 포인트 합계를 구하는 메서드의 이름 변경

1016027

코드 정렬작업 진행

138c4f9

Position의 getter, setter 수정 및 생성

bc50538

인덱스를 상징하는 변수를 생성하여 코드 변경

78ef6ea

인덱스를 상징하는 변수를 생성하여 코드 변경

3a2073d

totalPiecesCount 변수 이름 변경

032e838

내림차순 정렬을 위한 테스트 메서드 작성

b00feab

내림차순 정렬을 위한 테스트 메서드 작성

7157419

포인트 기준 정렬을 위하여 Comparable을 implements받아 메서드 작성

34c1686

중복을 제거하기 위하여 countAllPiecesByColor 메서드 생성

880b341

Rank를 담아두는 List의 이름 변경

d435cb2

전체적인 메서드명 점검 및 수정

0f641f9

LarryJung self-assigned this Feb 24, 2021

LarryJung self-requested a review February 24, 2021 16:08

LarryJung requested changes Feb 24, 2021

View reviewed changes

Jiwon-JJW added 4 commits February 25, 2021 22:24

countingAllPiecesByColor 코드 수정

8a07ec9

toString과 compareTo 메서드 밑으로 자리이동

de423b7

랭크 메서드로 piece.setPosition(chessBoardIndex) 명령어 이동으로 인한 수정

a0cb980

코드 정렬

59e7cf4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Starve] Mission5 - 보드에 위치 부여 및 점수 계산 #148

[Starve] Mission5 - 보드에 위치 부여 및 점수 계산 #148

Jiwon-JJW commented Feb 24, 2021

LarryJung left a comment

LarryJung Feb 24, 2021

LarryJung Feb 24, 2021

LarryJung Feb 24, 2021

LarryJung Feb 24, 2021

honux77 Mar 1, 2021

LarryJung Feb 24, 2021

Jiwon-JJW Feb 25, 2021

LarryJung Feb 24, 2021

Jiwon-JJW Feb 25, 2021

LarryJung Feb 24, 2021

LarryJung Feb 24, 2021

LarryJung Feb 24, 2021

LarryJung Feb 24, 2021

Jiwon-JJW Feb 25, 2021

		public int totalPiecesCount() {
		return countingAllPiecesByColor(Color.WHITE) + countingAllPiecesByColor(Color.BLACK);

[Starve] Mission5 - 보드에 위치 부여 및 점수 계산 #148

Are you sure you want to change the base?

[Starve] Mission5 - 보드에 위치 부여 및 점수 계산 #148

Conversation

Jiwon-JJW commented Feb 24, 2021

변경 사항

LarryJung 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