Skip to content
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

[사다리 미션] 신혜빈 미션 제출합니다. #20

Open
wants to merge 25 commits into
base: shin378378
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
3d3d769
docs : README파일 작성
shin378378 Oct 29, 2024
b85fa13
feat : 후보자 행 객체 생성
shin378378 Oct 29, 2024
e2342f2
feat : 행 객체 생성
shin378378 Oct 29, 2024
5c01a53
refactor : 사다리 객체 다시 구성
shin378378 Oct 29, 2024
ceb91b9
feat : Controller 생성
shin378378 Oct 29, 2024
b753e93
feat : inputView 생성
shin378378 Oct 29, 2024
651f22b
2단계 사다리 생성까지 완성
shin378378 Oct 29, 2024
9f1d637
feat : 참가자, 실행결과 객체 생성 기능 추가
shin378378 Oct 29, 2024
42c7d00
feat : 사다리 결과 출력 기능 추가
shin378378 Oct 30, 2024
14b4d35
4단계까지 완성
shin378378 Oct 30, 2024
210d7f3
test : 이름 글자수 오류 발생하는지 확인
shin378378 Oct 30, 2024
9739d30
docs : README 작성
shin378378 Oct 30, 2024
6422685
refactor : all을 입력받으면 멈추는 기능 do-while문으로 구현하기
shin378378 Nov 8, 2024
cd7c8a9
refactor : 특정 참가자의 결과 찾는 기능을 OutputView가 아닌 Participants로 옮기기
shin378378 Nov 8, 2024
f2642b3
refactor : 플레이어들과 결과들 생성하고 나서 리턴값 바꾸기
shin378378 Nov 8, 2024
62ded33
refactor : ladderRows를 Ladder객체로 입력받기
shin378378 Nov 8, 2024
b24b654
feat : enum 사용하기
shin378378 Nov 8, 2024
8b4800c
refactor : 불필요한 파라미터 없애기
shin378378 Nov 8, 2024
af2eec1
fix : README 수정하기
shin378378 Nov 13, 2024
e3d3190
feat : 00rl
shin378378 Nov 14, 2024
fa8e9e0
refactor : static 변수와 일반 변수는 한 줄 띄우기
shin378378 Nov 15, 2024
b21fef9
refactor : Players의 생성 방법 바꾸기
shin378378 Nov 15, 2024
d0a5f26
feat : PlayerResultDto 생성하기
shin378378 Nov 16, 2024
8a15278
refactor : Math.random()을 ThreadLocalRandom으로 수정하기
shin378378 Nov 16, 2024
37a8e9d
refactor : randomConnectedOrDisconnected로 메소드명 수정하기
shin378378 Nov 16, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# java-ladder

Choose a reason for hiding this comment

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

헉 리드미를 작성해주셔서 한 눈에 보기가 편하네요!


# < controller >
## 1) Controller
* createPlayers() //참여자 생성기능
* createResults() //참여결과 생성기능
* createLadder() //사다리 생성기능
* createLadderResult() //사다리 결과 생성기능
* playLadderGame() //사다리 게임 실행기능

Choose a reason for hiding this comment

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

자세하게 설명해주시려고 적어주신 리드미 너무 좋아요!!

개인적으로 리드미 작성에 추천드리는 방법이 있는데요
클래스 단위로 주석이나 설명을 적거나 패키지에 대한 설명을 적어주시는 방향을 통해서 작성해보시는 것도 많은 도움이 될 것 같아요!
메소드 단위의 설명은 변경 시점이 너무 잦다보니 리드미가 쉽게 레거시 코드같은 문서가 되기 정말 쉬울거에요

Copy link
Author

@shin378378 shin378378 Nov 12, 2024

Choose a reason for hiding this comment

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

저도 이번 문제를 푸면서 이 부분에 대한 고민이 많았습니다 ㅜㅠ
메소드는 계속 생성 수정 삭제가 되니까 README 관리가 너무 어렵더라구요.

좋은 조언감사합니다!! 앞으로는 구현한 기능에 대해 README를 작성하는 습관을 들여봐야겠어요!!

README 작성을 완료했는 데 여기서 더 자세히 적어야하는 지 덜 적어야하는 지는 감이 잘 안 잡히네요 ㅎㅎㅎ하하....
핵심 기능은 다 적어보려고 노렸했는 데 피드백 부탁드립니다!!

Copy link

@be-student be-student Nov 17, 2024

Choose a reason for hiding this comment

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

리드미를 남길 때, 주석을 남길때 모두 동일한데요
약간 메소드 레벨의 주석을 다는 것 보다는 클래스 레벨의 주석을 달아주면 좋을 것 같아요!
그리고 ~~~~ 하는 메소드 같이 코드를 보면 바로 파악할 수 있는 것 대신, 코드만으로 나타낼 수 없는 뭔가 특이한 조건이 있을 때만 주석을 달아주셔도 충분하긴 합니다!


---
# < model >
## 1) DecideResult
* moveColumn() // 열부분 움직이는 기능
* moveRow() //행부분 움직이는 기능
* decidePlayerResult() //참여자 결과 결정기능
## 2) Ladder
## 3) LadderRow
* LadderRow() //사다리 열부분 생성기능
* chooseTrueOrFalse() //사다리 point 정하는 기능
## 4) Player
* Player() //참여자 생성, 참가자이름이 5글자 초과 시 에러발생 기능
* getCertainPlayerResult //특정 사용자의 결과를 반환하는 기능
## 5) Players
## 6) Results
## 7) Splitter
* split() //문자열 자르는 기능

---
# < view >
## 1) InputView
* inputPlayers() //참여할 사람이름 입력기능
* inputResults() //결과 입력기능
* inputLadderHeight() //최대 사다리 높이 입력기능
* inputPlayerToWantResult() //결과를 보고싶은 사람 입력기능
## 2) OutputView
* outputPlayer() //참여자 출력기능
* outputLadderRow() //사다리행 출력기능
* outputLadder() //사다리 출력기능
* outputResults() //결과 출력기능
* outputAllPlayersResult //모든 참여자 결과 출력기능
* outputCertainPlayerResult //특정 참여자 결과 출력기능
* outputPlayerResult //참여자 결과 출력기능
8 changes: 8 additions & 0 deletions src/main/java/Main.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import controller.Controller;

public class Main {

Choose a reason for hiding this comment

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

저는 개인적으로는 Main 과 같은 클래스 네이밍을 별로 좋아하지 않는데요
비슷한 계열로 Controller 클래스도 별로 좋아하지 않아요
그 이유는 import 를 했을 때 이름이 겹칠 클래스가 엄청 많아서인데요
그래서 약간 길더라도 LadderController 같은 네이밍을 쓰는 것을 좋아하는데요 이 부분은 어떠신지 궁금합니다!

Copy link
Author

Choose a reason for hiding this comment

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

Controller는 여러개 일 수 있으니까 네이밍이 겹칠 수 있다는 건 생각 못 해본 관점인 거 같아요!
현재 LadderController로 네이밍 변경했습니다.

<질문>

Contoller가 여러개 일 수 있다는 건 이해가 갑니다.
하지만 public static void main이 들어가는 Main은 1개만 존재하는 데도 이름이 겹칠 수 있다는 게 잘 이해가 가지 않습니다.
Main이 겹치는 경우는 어떤 경우인가요?

Choose a reason for hiding this comment

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

이제 프로젝트가 커졌을 때 프로젝트를 실행시키는 main 함수를 찾기가 힘든데요
전체 검색으로 main(으로 검색해도 엄청 많이 나와요
이때 검색할 수 있는 키워드 역할을 파일명이 하는데요
LadderApplication 같은 네이밍의 파일이 있다면 그걸로 검색하고 갈 수 있어서 편한 것 같아요
프로젝트의 규모가 커졌을 때부터 큰 의미를 갖는 네이밍인 것 같아요

Copy link
Author

@shin378378 shin378378 Nov 13, 2024

Choose a reason for hiding this comment

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

Main은 무조건 하나밖에 없으니까 왜 Main이 여러개가 나온다는 건 지 몰랐는 데 하나의 어플리케이션 안에 LadderLotto 등 다양한 로직이 들어가면 main이 겹칠 수가 있군요!!

MainLadderApplication으로 변경해 주었습니다!!

public static void main(String[] args) {
Controller controller = new Controller();
controller.playLadderGame();
}
}
80 changes: 80 additions & 0 deletions src/main/java/controller/Controller.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
package controller;

import model.ladder.Ladder;
import model.ladder.LadderRow;
import model.player.Player;
import model.player.PlayerResults;
import model.player.Players;
import model.tool.Splitter;
import view.InputView;
import view.OutputView;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;


public class Controller {
InputView inputView = new InputView();
Splitter splitter = new Splitter();
OutputView outputView = new OutputView();

Choose a reason for hiding this comment

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

Input VIew 쪽에 달아드린 이 클래스가 static 으로 사용하지 않는 이유를 알고 싶어요!

Copy link
Author

Choose a reason for hiding this comment

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

앗 딱히 이유는 없고 빼먹는 거 같습니다!! 짚어주셔서 감사합니다 :)


private Players createPlayers() {
String playerNamesBeforeSplit = inputView.inputPlayers();
String[] playerNames = splitter.split(playerNamesBeforeSplit);

Choose a reason for hiding this comment

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

이 부분은 view 로 옮겨가면 좋을 것 같아요!
split 하는 부분이 controller에 있게 되면 점점 controller 의 복잡도가 올라갈 것 같아요

Copy link
Author

@shin378378 shin378378 Nov 14, 2024

Choose a reason for hiding this comment

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

제가 전에 문자열을 나누는 기능을 inputView에 만들었다가 객체는 그 역할에 충실해야한다
-> 즉, inputViewinput 받는 역할에만 충실해야한다 그래야 문제가 생겼을 때 빠르게 문제가 생긴 위치를 확인할 수 있다.
라는 피드백을 받았습니다.
저 역시 그 의견에 동의해서 inputView에는 Scanner에러발생 코드 빼고는 다른 코드를 넣지 않는 편입니다.

<질문>

private Players createPlayers() {
        String playerNamesBeforeSplit = inputView.inputPlayers();
        String[] playerNames = splitter.splitWithComma(playerNamesBeforeSplit);
        List<Player> playersInventory = new ArrayList<>();
        for (int i = 0; i < playerNames.length; i++) {
            Player player = new Player(i, playerNames[i]);
            playersInventory.add(player);
        }
        Players players = new Players(playersInventory);
        return players;
    }

split은 그대로 놔두고 Players의 생성자가 더 많은 역할을 하도록 위의 코드를 아래의 코드로 바꿔보았는 데 혹시 이 방법은 어떠신가요..??

private Players createPlayers() {
        String playerNamesBeforeSplit = inputView.inputPlayers();
        String[] playerNames = splitter.splitWithComma(playerNamesBeforeSplit);
        Players players = new Players(playerNames);
        return players;
    }

Choose a reason for hiding this comment

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

오 이정도로 바뀐다면 괜찮을 것 같아요!

-> 즉, inputView는 input 받는 역할에만 충실해야한다 그래야 문제가 생겼을 때 빠르게 문제가 생긴 위치를 확인할 수 있다.

이 부분은 진짜 취향에 따라 정말 많이 갈리는 부분이라서 그렇게 생각하신다면 그렇게 해주시면 됩니다!

List<Player> playersInventory = new ArrayList<>();
for (int i = 0; i < playerNames.length; i++) {
Player player = new Player(i, playerNames[i]);
playersInventory.add(player);
}
Players players = new Players(playersInventory);
return players;
}

private List<String> createLadderResults() {
String trialResultsBeforeSplit = inputView.inputResults();
String[] trialResults = splitter.split(trialResultsBeforeSplit);
List<String> ladderResults = Arrays.asList(trialResults);
return ladderResults;
}

private Ladder createLadder(int columnSize) {
List<String> ladderResults = createLadderResults();
int rowSize = inputView.inputLadderHeight();
List<LadderRow> ladderRows = new ArrayList<>();
for (int i = 0; i < rowSize; i++) {
LadderRow ladderRow = new LadderRow(columnSize);
ladderRows.add(ladderRow);
}
Ladder ladder = new Ladder(ladderRows, ladderResults);
return ladder;
}

private PlayerResults createPlayerResults(Players players, Ladder ladder) {
List<Player> playerInventory = players.getPlayerInventory();
Map<String, String> playerResultsInventory = ladder.decidePlayerResults(playerInventory);
PlayerResults playerResults = new PlayerResults(playerResultsInventory);
return playerResults;
}

private void printLadder(Players players, Ladder ladder) {
List<LadderRow> ladderRows = ladder.getLadderRows();
List<String> ladderResults = ladder.getLadderResults();
List<Player> playerInventory = players.getPlayerInventory();
OutputView outputView = new OutputView();
outputView.outputLadder(playerInventory, ladderRows, ladderResults);
}

public void playLadderGame() {
Players players = createPlayers();
Ladder ladder = createLadder(players.getPlayerInventory().size() - 1);
PlayerResults playerResults = createPlayerResults(players, ladder);
printLadder(players, ladder);
String playerName;
do {
playerName = inputView.inputPlayerToWantResult();
outputView.outputPlayerResult(playerResults, playerName);
} while (!playerName.equals("all"));
}
}
60 changes: 60 additions & 0 deletions src/main/java/model/ladder/Ladder.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package model.ladder;

import model.player.Player;

import java.util.HashMap;
import java.util.List;
import java.util.Map;

public class Ladder {
private List<LadderRow> ladderRows;
private List<String> ladderResults;

public Ladder(List<LadderRow> ladderRows, List<String> ladderResults) {
this.ladderRows = ladderRows;
this.ladderResults = ladderResults;
}

private LadderMove changePosition(List<Boolean> points, int columnPosition) {
if (columnPosition > 0 && points.get(columnPosition - 1)) {
return LadderMove.LEFT;
}
if (columnPosition < points.size() && points.get(columnPosition)) {
return LadderMove.RIGHT;
}
return LadderMove.DOWN;
}

private int moveColumn(List<Boolean> points, int columnPosition) {
LadderMove moveDirection = changePosition(points, columnPosition);
return columnPosition + moveDirection.getOffset();
}

private int moveRow(List<LadderRow> ladderRows, int columnPosition) {
for (LadderRow ladderRow : ladderRows) {
List<Boolean> points = ladderRow.getPoints();
columnPosition = moveColumn(points, columnPosition);
}
return columnPosition;
}

public Map<String, String> decidePlayerResults(List<Player> playerInventory) {
Map<String, String> playerResultsInventory = new HashMap<>();
for (Player player : playerInventory) {
int playerPosition = player.getPosition();
int columnPosition = moveRow(ladderRows, playerPosition);
String playerName = player.getName();
String playerResult = ladderResults.get(columnPosition);
playerResultsInventory.put(playerName, playerResult);
}
return playerResultsInventory;
}

public List<LadderRow> getLadderRows() {
return ladderRows;
}

public List<String> getLadderResults() {
return ladderResults;
}
}
17 changes: 17 additions & 0 deletions src/main/java/model/ladder/LadderMove.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package model.ladder;

public enum LadderMove {

Choose a reason for hiding this comment

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

이 부분까지 enum 으로 두기 쉽지 않았을텐데 분리하려고 열심히 하신 것이 보여서 되게 좋네요!

Copy link
Author

Choose a reason for hiding this comment

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

감사합니다!!
굳이 쓸 필요 없는 부분같긴 한 데 enum사용을 경험해본 다는 것에 의의를 두고 적어보았습니당 :)

RIGHT(1),
LEFT(-1),
DOWN(0);

private final int offset;

LadderMove(int offset) {
this.offset = offset;
}

public int getOffset() {
return offset;
}
}
16 changes: 16 additions & 0 deletions src/main/java/model/ladder/LadderPoint.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package model.ladder;

public enum LadderPoint {
CONNECTED(true),
DISCONNECTED(false);

private final Boolean isConnected;

LadderPoint(Boolean isConnected) {
this.isConnected = isConnected;
}

public Boolean getConnected() {
return isConnected;
}
}
31 changes: 31 additions & 0 deletions src/main/java/model/ladder/LadderRow.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package model.ladder;

import java.util.ArrayList;
import java.util.List;

public class LadderRow {
private List<Boolean> points = new ArrayList<>();

public LadderRow(int columnSize) {
for (int columnPosition = 0; columnPosition < columnSize; columnPosition++) {
boolean randomBoolean = chooseLadderPoint(columnPosition);
this.points.add(columnPosition, randomBoolean);
}
}

private LadderPoint randomTrueOrFalse() {

Choose a reason for hiding this comment

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

이 메소드의 경우에 randomTrueOrFalse 라는 네이밍을 봤을 때는 뭔가 boolean 타입을 반환해야 할 것 같은데 어떻게 생각하시나요?

Copy link
Author

Choose a reason for hiding this comment

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

앗! 생각 못 해봤는 데 randomTrueOrFalse가 반환값을 헷갈리게 하는 군요!
randomConnectedOrDisconnected()로 메소드명을 바꾸어 좀 더 반환값에 적합한 네이밍으로 변경하였습니다 :)

if (Math.random() < 0.5) return LadderPoint.CONNECTED;

Choose a reason for hiding this comment

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

https://morethantoday.tistory.com/94
자바에는 random 을 쓰는 것이 총 4가지가 있어요!
Math.random(), ThreadLocalRandom, SecureRandom, Random()
이중에서 보통 ThreadLocalRandom, SecureRandom 쪽만 실무에서는 써왔던 것 같아요!

Copy link
Author

Choose a reason for hiding this comment

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

Math.random()을 실무에서는 쓰지 않는다니... 몰랐던 사실이군요!
물론 지금 제 수준에서는 ThreadLocalRandom이 굳이 필요없을 거 같긴 하지만 더 오래 기억에 남기기 위해 아래와 같이 한 번 수정해보았습니다!

private LadderPoint randomTrueOrFalse() {
        if (ThreadLocalRandom.current().nextDouble() < 0.5) return LadderPoint.CONNECTED;
        return LadderPoint.DISCONNECTED;
    }

Choose a reason for hiding this comment

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

넵 좋아요! 👍

return LadderPoint.DISCONNECTED;
}

public boolean chooseLadderPoint(int columnPosition) {
if (columnPosition > 0 && points.get(columnPosition - 1))
return LadderPoint.DISCONNECTED.getConnected();
return randomTrueOrFalse().getConnected();

}

public List<Boolean> getPoints() {
return points;
}
}
23 changes: 23 additions & 0 deletions src/main/java/model/player/Player.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package model.player;

public class Player {
private static final int NAME_LENGTH_LIMIT = 5;

Choose a reason for hiding this comment

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

자바 컨벤션 상으로 static 변수와 일반 변수는 1줄을 띄워야 할 것 같아요!

Copy link
Author

Choose a reason for hiding this comment

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

앗 static 변수와 일반 변수는 1줄을 띄어야 하는 군요! 수정하겠습니다!!
이런 디테일까지 알려주셔서 감사합니다 :)

private int position;
private String name;

public Player(int position, String name) {
if (name.length() > NAME_LENGTH_LIMIT) {
throw new IllegalArgumentException("참가자 이름이 " + NAME_LENGTH_LIMIT + "자 초과입니다.");
}
this.position = position;
this.name = name;
}

public int getPosition() {
return position;
}

public String getName() {
return name;
}
}
19 changes: 19 additions & 0 deletions src/main/java/model/player/PlayerResults.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package model.player;

import java.util.Map;

public class PlayerResults {

Choose a reason for hiding this comment

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

바로 전에 말씀드린 Players 와는 반대로 이 클래스는 충분히 있어도 된다고 생각하는 편이긴 한데요
dto 의 경우에는 직접 Map<String, String> 형태로 넘기게 되면 이 클래스는 뭘 의미하는거지? 라는 것을 헷갈려 할 수도 있을 것 같아요
그래서 dto 에 경우에는 예외적으로 아무 메소드가 없어도 클래스로 만드는 것도 나쁘지 않다고 생각하는 편입니다!

Copy link
Author

@shin378378 shin378378 Nov 16, 2024

Choose a reason for hiding this comment

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

이번 기회를 통해 Map과 DTO의 차이점에 대해 알아보았습니다!


  1. 가독성
    DTO: 클래스와 필드로 구성되어 있으며, 필드가 어떤 형태의 데이터인 지, 어떤 데이터를 나타내는지 명확히 알 수 있다.
    Map: 키가 문자열이나 객체로 되어있어서 데이터의 구조가 복잡해지면 각 키의 의미를 파악하기 어렵다.

  2. 유연성
    DTO: 미리 정의된 클래스이므로, 데이터 구조를 변경하려면 DTO 클래스를 수정해야한다.
    Map: 동적으로 키를 추가하거나 삭제할 수 있어서 매우 유연하게 데이터를 다룰 수 있다.

  3. 타입 안정성
    DTO: 각 필드의 데이터 타입이 명확하게 지정되기 때문에 컴파일 타임에 타입 검사가 이루어져서 잘못된 타입의 데이터가 할당되면 에러를 발생시키므로 타입 안전성을 보장한다.
    Map : 값을 저장할 때 키와 관련된 타입 정보를 명시하지 않아도 되기 때문에 특정 키에 잘못된 타입의 값을 저장해도 컴파일 타임에서는 검사가 이루어지지 않으며, 런타임에 타입 에러가 발생할 가능성이 있다.

→ 결론:
데이터가 안전해야하고, 오류 검사가 빨라야하고, 고정된 데이터 구조가 필요할 때는 DTO를 사용하고
유연한 대처가 필요하고 데이터 구조가 고정되지 않았을 때는 Map을 사용하는 것이 좋다.


장단점을 비교해놓고 보니 확실히 프로그램이 커질수록 정보관리에는 DTO를 쓰는 게 더 적합하다는 생각이 듭니다!!
덕분에 Map과 DTO에 대해서 공부해볼 수 있는 기회가 생겼습니다! 감사합니다 :)

private Map<String, String> PlayerResultsInventory;

public PlayerResults(Map<String, String> playerResultsInventory) {
PlayerResultsInventory = playerResultsInventory;
}

public String getPlayerResult(String playerName) {
return PlayerResultsInventory.get(playerName);
}

public Map<String, String> getPlayerResultsInventory() {
return PlayerResultsInventory;
}
}
15 changes: 15 additions & 0 deletions src/main/java/model/player/Players.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package model.player;

import java.util.List;

public class Players {

Choose a reason for hiding this comment

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

일급 컬렉션을 지키려고 노력하신 부분이 보여서 고생 많으셨다는 생각이 드네요
이 클래스의 경우에는 method 가 오직 getter 하나 뿐인데요 이렇게 되었을 경우에 일급 컬렉션의 역할을 하는 것 대신 클래스의 숫자가 늘어나서 코드를 읽기 힘들어질 수도 있을 것 같아요!
저는 개인적으로는 여기에 player 숫자에 대한 검증과 같은 무엇인가가 없다면 이 클래스는 없어도 된다는 생각입니다!

Copy link
Author

@shin378378 shin378378 Nov 15, 2024

Choose a reason for hiding this comment

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

< 질문 >

컨트롤러의 역할을 줄이기 위해 아래와 같이 Player객체 생성을 Controller가 아닌 Players가 하도록 로직을 변경해주었습니다.
Players의 생성자가 특별한 역할(객체 생성)을 수행하더라도, 여전히 method가 getter 하나뿐이라면 피드백과 동일한 의견인지 궁금합니다 ㅜㅠ
사실 저도 Players가 getter외에 다른 역할을 하지 않는다는 것을 인지하고 있었지만 그렇다면 Player의 모음체인 List<Player> PlayerInventory는 대체 어디서 관리해야하는지 감이 안 잡힙니다 ㅜㅠ

public Players(String [] playerNames){
        for (int i = 0; i < playerNames.length; i++) {
            Player player = new Player(i, playerNames[i]);
            playerInventory.add(player);
        }
    }

Copy link

@be-student be-student Nov 17, 2024

Choose a reason for hiding this comment

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

만약 그렇게 역할이 딱히 없다면 list 형태로 들고다녀도 저는 좋다고 생각합니다!
일단 지금의 코드를 기준으로 봤을 때, list 형태로 들고다니는 것과, 일급 컬렉션을 쓰는 것 그 2가지가 전혀 차이가 없다보니.... 오히려 일급컬렉션을 하다보면 코드 이해하기가 어려워지는 것 같아요

private List<Player> playerInventory;

public Players(List<Player> playerInventory) {
this.playerInventory = playerInventory;
}

public List<Player> getPlayerInventory() {
return playerInventory;
}
}
13 changes: 13 additions & 0 deletions src/main/java/model/tool/Splitter.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package model.tool;

public class Splitter {
final static private String SPLIT_SIGN = ",";

public String[] split(String beforeSplit) {

Choose a reason for hiding this comment

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

다른분 리뷰에서 남겼던 내용인데요
이 부분은 분명히 유틸리티 함수를 이용하는 느낌이 드는데요
만약 진짜 유틸리티 함수라면 이런 형태로 시그니처가 바뀌는 것이 더 좋을 것 같아요
split(String beforeSplit, String tokenizer)
이나
splitWithComma(String beforeSplit)

이런 형태로 바뀌게 되면 쓰는 입장에서는 조금 더 생각하지 않고 쓸 수 있을 것 같아요
이전 코드의 경우에는 어떤 구분자로 자르지? 라는 것을 확인하러 와야 할 것 같아서요

Copy link
Author

Choose a reason for hiding this comment

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

split이란 이름이 "대체 뭘 split한다는 거지?"라는 의문을 남길 수도 있군요 :)
splitWithComma라는 직관적인 이름으로 대체해주었습니다!!

String[] afterSplit = beforeSplit.split(SPLIT_SIGN);
for (int i = 0; i < afterSplit.length; i++) {
afterSplit[i] = afterSplit[i].trim();
}
return afterSplit;
}
}
26 changes: 26 additions & 0 deletions src/main/java/view/InputView.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package view;

import java.util.Scanner;

public class InputView {
Scanner scanner = new Scanner(System.in);

Choose a reason for hiding this comment

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

다른 부분의 구현을 정말 잘 해주신 것 같아서
조금 디테일하게 챙길 수 있을 것 같을 부분을 말씀드리려고 하는데요
혹시 이 클래스가 Static method 로 이루어지지 않은 이유가 있을까요?
private static final Scanner scanner = new Scanner(System.in); 으로 바꾸면 현재 있는 것들을 전부 static 으로 바꿀 수 있을테니까요

Copy link
Author

@shin378378 shin378378 Nov 12, 2024

Choose a reason for hiding this comment

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

딱히 이유는 없습니다 ㅜㅠ
Controller 안에서 inputViewoutputViewstatic을 붙여야 한다는 개념은 생각해봤는 데 그 안의 Scanner에도 static을 붙여줄 수 있다는 개념은 생각 못 해본 거 같아요! 짚어주셔서 감사합니다!!

현재 스캐너와 InputView OutputView의 모든 메서드에 static 을 붙여주었습니다!

<질문>

static을 붙이는 이유가 "이 값은 객체마다의 특징을 나타내는 값이 아니다"라는 특징을 명시해주기 위해서 인가요?
아니면 또 다른 이유가 있는 건가요?
미리 값들을 한 번에 불러온다는 게 좋은 건지 잘 모르겠습니다 ㅜㅠ
미리 불러오더라도 그 값을 안 쓰게 되면 괜한 메모리 낭비가 아닌가 하는 생각이 있기 때문입니다.

Choose a reason for hiding this comment

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

보통 스태틱으로 할당되는 메모리는 신경쓸 필요가 없어요 (map 에 10만개쯤 넣는 순간부터 고민해도 충분합니다)
그래서 그 이유는 아니고요

스태틱을 쓰면 가장 간단하게는 생성자가 간단해지잖아요?
테스트에서 그 객체를 다룰 필요가 없을 때는 생성자가 간단해져서 테스트가 오히려 쉬워지기도 합니다
물론 수기 구현으로 테스트에서 모킹된 객체를 넣는다면 그때부터는 static이 아닌 쪽이 편합니다
모든 것에는 장단이 있어요!

Copy link
Author

@shin378378 shin378378 Nov 13, 2024

Choose a reason for hiding this comment

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

staic이라는 게 생각보다 메모리를 많이 잡아먹진 않군요 :) 몰랐던 지식인 데 알려주셔서 감사합니다!

생성자가 간단해진다는 생각은 못 해봤어요!! 생각해보지 못 한 관점이네요.
그런데 꼭 static이 아니여도 아래와 같이 선언해두면 생성자가 간단해지지 않나요?
static으로 변수를 미리 선언해주는 것과 아래와 같이 변수를 설정해주는 것의 차이가 모호하게 느껴져요...

private String name = "아무개";

Choose a reason for hiding this comment

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

만약 그렇게 된다면 큰 차이는 없을 것 같아요!
Java 기준에서는 Lombok 으로 생성자를 만드는 경우가 많은데, 이때 기준으로 봤을 때 @RequiredArgsConstructor 를 쓰지 못한다는 것이 약간 아쉬울 수 있는데, 이 부분만 알고서 쓰신다면 문제는 없을 것 같아요

Copy link
Author

Choose a reason for hiding this comment

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

@RequiredArgsConstructor는 처음 들어 봤는 데 생성자를 초기화 해주는 기능이군요:)
static 변수는 생성자와 관련이 없어서 @RequiredArgsConstructor로 처리할 수 없다는 점은 이번 기회로 잘 인지하고 있겠습니다!! 감사합니다!!

public String inputPlayers(){
System.out.println("참여할 사람 이름을 입력하세요. (이름은 쉼표(,)로 구분하세요)");
return scanner.nextLine();
}

public String inputResults(){
System.out.println("\n"+"실행 결과를 입력하세요. (결과는 쉼표(,)로 구분하세요)");
return scanner.nextLine();
}

public Integer inputLadderHeight(){
System.out.println("\n"+"최대 사다리 높이는 몇 개인가요?");
return Integer.parseInt(scanner.nextLine());
}

public String inputPlayerToWantResult(){
System.out.println("\n"+"결과를 보고 싶은 사람은?");
return scanner.nextLine();
}
}
Loading