Skip to content

사다리 2단계 구현 #6

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

Merged
merged 14 commits into from
Nov 19, 2018
Merged

사다리 2단계 구현 #6

merged 14 commits into from
Nov 19, 2018

Conversation

lalwr
Copy link

@lalwr lalwr commented Nov 17, 2018

No description provided.

Copy link
Contributor

@javajigi javajigi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

테스트하기 쉬운 구조로 설계를 개선해 보면 좋겠어요.

@@ -8,7 +8,7 @@
private List<Player> players;
private List<Line> lines;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

일급 콜렉션을 적용하라는 원칙에 따라 List<Player> playersList<Line> lines을 새로운 객체로 래핑하는 것은 어떨까?

return location >= points.size() ? false : points.get(location);
}

public int Move(int location) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

메소드 이름은 소문자

@@ -13,4 +14,22 @@ public Line(List<Boolean> points) {
public List<Boolean> getPoints() {
return points;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

List를 외부에 공개하는 경우 외부에서 수정/삭제할 위험성이 있다.
따라서 외부에 공개할 때는 수정하지 못하도록 java.util.Collections.unmodifiableList() 사용해 반환할 것을 추천한다.

return new PlayResult(player, result);
}

private List<PlayResult> AllPlayerResult(LadderResult ladderResult) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

메소드 이름은 소문자

List<PlayResult> playResults;
if(playerName.equals(ALL_USER)){
playResults = AllPlayerResult(ladderResult);
}else{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

else를 쓰지 않고 구현한다.


private PlayResult setResult(Player player, String result) {
return new PlayResult(player, result);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

굳이 이런 부분까지 메소드로 분리하는 것은 너무 과한 느낌이다.

LadderResult ladderResult = new LadderResult(result);
resultView.result(ladderManage, ladderResult);

Laddering laddering = new Laddering(ladderManage.getPlayers(), ladderManage.getLines());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Laddering laddering = ladderManage.play(playerNames)와 같은 구조로 개발하면 어떨까?


public class LadderingTest {
@Test
public void 사다리결과() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

테스트 메소드가 너무 복잡하다.
좀 더 단순하게 테스트할 수 있도록 객체 설계를 변경해 본다.

@javajigi javajigi merged commit 67ef1b8 into next-step:lalwr Nov 19, 2018
pcspapa added a commit to pcspapa/java-ladder that referenced this pull request Aug 28, 2020
parkeeseul pushed a commit that referenced this pull request Sep 3, 2020
* refactor: 리뷰내용 반영

메소드명 동사화
사다리 폭 테스트

* refactor: 모든 원시값과 문자열을 포장한다.

Boolean to Point

* feat: Line 에서의 분석기 (#7)

Closes #7

* refactor: Point 클래스 position 필드 추가에 대한 기존 클래스 변경

* refactor: 모든 원시값과 문자열을 포장한다.

현재 위치 값 index 를 Chessmen 으로 포장함.

* feat: Ladder 에서의 분서기

Closes #6

* feat: 실행 결과 입력하기

Closes #8

* feat: 결과 요청 메시지

Closes #12

* bugfix: LineAnalysis 버그

LineAnalysis 에서 상태 계산 버그.
LineAnalysis 에서 속도 개선을 위한 선택가능한 라인만 추출

* feat: 결과 요처어

1. 1인의 결과 요청
1. 모두의 결과 요청

Closes #9, #10

* refactor: 실행결과 유효성 체크

* feat: 1. 실행 결과 출력하기

Closes #12

* feat: 사다리에 결과값 출력하기

Closes #13

* feat: 일급 일급컬렉션으로 - Players

Closes #14

* feat: 일급 일급컬렉션으로 - Prizes

Closes #15

* feat: 결과를 만드는 행위를 Line, Ladder 에서 하도록 수정하자.

Closes #16

* refactor: Analysis class 삭제 및 중복 제거

* refactor: 리뷰내용 반영
nno0obb pushed a commit to nno0obb/java-ladder that referenced this pull request Apr 7, 2025
testrace pushed a commit that referenced this pull request Apr 10, 2025
* [ladder] step2: .gitignore

* [ladder] step2: README.md

* [ladder] step1: feedback #1

* [ladder] step1: feedback #2

* [ladder] step1: feedback #3

* [ladder] step2: README.md

* [ladder] step2: init

* [ladder] step2: Height

* [ladder] step2: Name

* [ladder] step2: PointX

* [ladder] step2: Line

* [ladder] step2: NameList

* [ladder] step2: README.md

* [ladder] step2: Ladder

* [ladder] step2: PointX

* [ladder] step2: Board

* [ladder] step2: Typo

* [ladder] step2: Typo

* [ladder] step2: Typo

* [ladder] step2: README.md

* [ladder] step2: feedback #1

* [ladder] step2: feedback #2,3

* [ladder] step2: feedback #4,8

* [ladder] step2: README.md

* [ladder] step2: #5

* [ladder] step2: #6,7

* [ladder] step2: README.md

* [ladder] step2: README.md

* [ladder] step2: #9

* [ladder] step2: #10

* [ladder] step2: README.md

* [ladder] step2: Point, Bridge

* [ladder] step2: README.md

* [ladder] step2: feedback #1

* [ladder] step2: feedback #2,3,4,5,6,7

---------

Co-authored-by: bt.7274(김홍균)/kakao <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants