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

사다리-함수형 프로그래밍 #22

Open
wants to merge 13 commits into
base: sansan20535
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
Empty file removed src/main/java/.gitkeep
Empty file.
23 changes: 23 additions & 0 deletions src/main/java/LadderApplication.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import controller.LadderController;
import repository.ladder.LadderRepository;
import repository.results.ResultsRepository;
import repository.users.UserRepository;
import service.ladder.LadderServiceImpl;
import service.results.ResultsServiceImpl;
import service.users.UserServiceImpl;
import view.InputView;
import view.OutputView;

public class LadderApplication {
public static void main(String[] args) {
LadderController ladderController = new LadderController(
new LadderServiceImpl(new LadderRepository()),
new UserServiceImpl(new UserRepository()),
new ResultsServiceImpl(new ResultsRepository()),
new OutputView(),
new InputView()
);

ladderController.start();
}
}
82 changes: 82 additions & 0 deletions src/main/java/controller/LadderController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
package controller;

import domain.ladder.Ladder;
import domain.ladder.LadderLine;
import domain.results.Results;
import domain.users.User;
import domain.users.Users;
import service.ladder.LadderService;
import service.results.ResultsService;
import service.users.UserService;
import view.InputView;
import view.OutputView;

public class LadderController {

private final LadderService ladderService;
private final UserService userService;
private final ResultsService resultsService;
private final OutputView outputView;
private final InputView inputView;

public LadderController(final LadderService ladderService, final UserService userService, final ResultsService resultsService, final OutputView outputView, final InputView inputView) {
this.ladderService = ladderService;
this.userService = userService;
this.resultsService = resultsService;
this.outputView = outputView;
this.inputView = inputView;
}

public void start() {
userService.makeUsers(inputView.inputUserNames());
resultsService.makeResults(inputView.inputResults());
ladderService.makeLadder(userService.getUsers().users().size(), inputView.inputHeight());

userService.updatePosition(ladderService.getLadder(), userService.getUsers());

printLadder(userService.getUsers(), ladderService.getLadder(), resultsService.getResults());
printLadderResult(userService.getUsers(), resultsService.getResults());
}

private void printLadder(final Users users, final Ladder ladder, final Results results) {
for (User user : users.users()) {
outputView.printUser(user.getName());
}
outputView.printEmpty();
for (LadderLine ladderLine : ladder.ladderLines()) {
outputView.printLadderLine(ladderLine.line());
}
for (String result : results.results()) {
outputView.printResult(result);
}
outputView.printEmpty();
}

private void printLadderResult(final Users users, final Results results) {
while (true) {

Choose a reason for hiding this comment

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

while(true) 로 되어있는 구문은 break 문을 무조건 바로 보이도록 적어주시면 좋을 것 같아요
그리고 while(true) 의 경우에는 break 가 있더라도 무한 루프가 될 수 있다는 것때문에 최대한 지양하는 것이 좋을 것 같아요

Copy link
Author

Choose a reason for hiding this comment

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

따로 종료 조건을 명시한 부분을 찾을 수 없어서 break와 같은 반복 종료문을 설정하지 않았습니다..!! 또한 횟수가 정해지지 않아서 while(true)를 사용했었는데 혹시 추천하시는 방법이 있으신가요?!

Choose a reason for hiding this comment

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

이 부분의 경우에는 최대 반복 횟수가 사다리 최대 길이로 볼 수 있을 것 같아요!
직접 정의한 숫자를 최대 길이로 잡아도 좋을 것 같아요!

final String wantedUserName = inputView.inputWantedUserName();
final boolean isAll = checkUserName(wantedUserName);
printResult(wantedUserName, users, results, isAll);
}
}

private boolean checkUserName(final String wantedUserName) {

Choose a reason for hiding this comment

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

1줄정도의 코드의 경우에는 함수로 분리하지 않는 것이 좋을 것 같아요!
물론 indent 룰을 적용해보시는 것은 아주 좋다고 생각합니다

1줄이 함수가 되려면 맥락이 코드로 나타낼 수가 없는 경우가 많은데요 함수의 이름을 명확하게 짓거나 주석을 통해서 자세한 설명을 왕창 달아주어야 하는 경우에만 이렇게 쓰고 있어요

Copy link
Author

Choose a reason for hiding this comment

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

아하...!! 안 그래도 최대한 기능을 쪼갠다는 생각때문에 함수를 많이 분리하는 과정에서 나중에 다른 팀원이 봤을 땐 "이 함수는 또 뭐지"라는 상황이 올 수도 있을 것 같다고 생각했던 참이었습니다! 감사합니다 : )

return wantedUserName.equals("all");
}

private void printResult(final String wantedUserName, final Users users, final Results results, final boolean isAll) {
if (!isAll) {
outputView.printExecutionResult();
outputView.printUserResult(results.results()
.get(userService.findByName(wantedUserName).getPosition())
);
return;
}

outputView.printExecutionResult();
for (User user : users.users()) {
outputView.printAllUserResults(user.getName(), results.results()
.get(user.getPosition()));
}
}
}
9 changes: 9 additions & 0 deletions src/main/java/domain/ladder/Ladder.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package domain.ladder;

import java.util.List;

public record Ladder(
List<LadderLine> ladderLines,
LadderInfo ladderInfo
) {
}
7 changes: 7 additions & 0 deletions src/main/java/domain/ladder/LadderInfo.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package domain.ladder;

public record LadderInfo(
int width,
int height
) {
}
9 changes: 9 additions & 0 deletions src/main/java/domain/ladder/LadderLine.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package domain.ladder;

import java.util.List;

public record LadderLine(
List<Boolean> connections,
String line
) {
}
28 changes: 28 additions & 0 deletions src/main/java/domain/ladder/LadderLineConnection.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package domain.ladder;

import java.util.Arrays;

public enum LadderLineConnection {

CONNECT_LADDER("|-----", true),
NOT_CONNECT_LADDER("| ", false);

LadderLineConnection(final String ladderConnectionFormat, final boolean isConnected) {
this.ladderConnectionFormat = ladderConnectionFormat;
this.isConnected = isConnected;
}

private final String ladderConnectionFormat;
private final boolean isConnected;

public String getLadderConnectionFormat() {
return ladderConnectionFormat;
}

public static LadderLineConnection of(final boolean isConnected) {
return Arrays.stream(values())
.filter(ladderLineConnection -> ladderLineConnection.isConnected == isConnected)
.findAny()
.orElseThrow();
}
}

Choose a reason for hiding this comment

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

물론 이렇게 stream 을 쓰는 것도 학습의 목적에서는 좋습니다!
하지만 그냥 이렇게 해도 보는 입장에서 편할 것 같아요
if(isConnected){
return CONNECT_LADDER;
}
return NOT_CONNECT_LADDER;

Copy link
Author

Choose a reason for hiding this comment

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

여기서는 true, false 두 가지 경우만 있다는 사실을 간과했던 것 같습니다! 감사합니다 : )

9 changes: 9 additions & 0 deletions src/main/java/domain/results/Results.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package domain.results;

import java.util.List;

public record Results(
List<String> results
) {

}
25 changes: 25 additions & 0 deletions src/main/java/domain/users/User.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package domain.users;

public class User {

private final String name;
private int position;

public User(final String name, final int position) {
this.name = name;
this.position = position;
}

public String getName() {
return name;
}

public void setPosition(int position) {
this.position = position;
}

public int getPosition() {
return position;
}

}
8 changes: 8 additions & 0 deletions src/main/java/domain/users/Users.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package domain.users;

import java.util.List;

public record Users(

Choose a reason for hiding this comment

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

약간 개인 취향에 가까운데요
가이드라인에서는 분명히 일급 컬렉션을 되게 중요시 여기는 것을 알고 있어서 그 부분을 적용해주시는 것은 아주 좋다고 생각합니다!
해봐야 어떤 부분이 좋은지 명확하게 알다보니까요

단순하게 .users() 형태로 꺼내오기만 하게 되면 이 클래스의 존재 의미가 없을 것 같아요

만약 이 클래스를 쓰게 된다면 조금 더 메소드를 넣어서 이 클래스의 기능을 추가해보는 것이 좋을 것 같아요

Copy link
Author

Choose a reason for hiding this comment

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

맞는 것 같습니다! 여기서는 "User의 모음"이라는 부분을 강조하기 위해 Users를 만들었는데, 이후 유저에 관한 작업이 명시되지 않는 다면 리팩토링하면 좋을 것 같습니다 !! 감사합니다 : )

List<User> users
) {
}
16 changes: 16 additions & 0 deletions src/main/java/repository/ladder/LadderRepository.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package repository.ladder;

import domain.ladder.Ladder;

public class LadderRepository {

Choose a reason for hiding this comment

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

저도 이 비슷한 코드를 프리코스중 하나에 만들었던 기억이 있는데요
참고차 전달드리고 싶은 내용이 있는데요 Repository 를 넣는 것은 약간 주의를 해야하는 부분입니다 (적어주신 것처럼 그냥 써보고 싶으신 것이다보니 그냥 무시하셔도 됩니다!)
Repository 라는 것은 db 에 저장하게 되면 영구히 저장되는 기능을 잘 사용하는 느낌이 되지만
이런 형태로 콘솔 애플리케이션에 경우에는 그냥 전역변수의 역할만 하게 됩니다
(전역변수다보니 나중에 봤을 때 이 코드는 어디서 불리는건지를 모르는 유지보수에 힘든 점들이 있더라고요...)

Copy link
Author

Choose a reason for hiding this comment

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

아하..!! 그런 불편함이 있을 수 있군요..!! 감사합니다 다음부터 유의해보도록 하겠습니다 : )


private Ladder ladder;

public Ladder getLadder() {
return ladder;
}

public void setLadder(final Ladder ladder) {
this.ladder = ladder;
}
}
16 changes: 16 additions & 0 deletions src/main/java/repository/results/ResultsRepository.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package repository.results;

import domain.results.Results;

public class ResultsRepository {

private Results results;

public Results getResults() {
return results;
}

public void setResults(final Results results) {
this.results = results;
}
}
65 changes: 65 additions & 0 deletions src/main/java/repository/users/UserRepository.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
package repository.users;

import domain.ladder.LadderInfo;
import domain.ladder.LadderLine;
import domain.users.User;
import domain.users.Users;

import java.util.List;

public class UserRepository {

private Users users;

public Users getUsers() {
return users;
}

public void setUsers(final Users users) {
this.users = users;
}

public User findByName(final String userName) {
return users.users().stream()
.filter(user -> user.getName().equals(userName))
.findFirst()
.orElse(null); // 또는 예외를 던질 수 있습니다.

Choose a reason for hiding this comment

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

저는 이 부분이 가장 중요한 부분이라고 생각하는데요
orElse 에서 어떤 동작을 하는지가 외부에서 repository 를 쓸 때 가장 주의를 하는 부분이다보니 @nullable 같이 명확하게 이 메소드는 null 을 반환하니까 그냥 믿고 쓰세요 라고 해주시는 방향이 외부에서 사용하기 편할 것 같아요

Copy link
Author

Choose a reason for hiding this comment

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

아하..! 그 점까지는 생각하지 못했던 것 같습니다..!! 참고하도록 하겠습니다 감사합니다 : )

}

public void updatePosition(final User user, final List<LadderLine> ladderLines, final LadderInfo ladderInfo) {

Choose a reason for hiding this comment

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

repository 다보니 이 부분은 service 레이어로 가면 좋을 것 같아요!

Copy link
Author

Choose a reason for hiding this comment

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

말씀하신 대로 이 로직은 service의 역할이 맞는 것 같습니다!! 감사합니다 : )

for (LadderLine ladderLine : ladderLines) {
final boolean isConnectedRight = checkRightConnection(ladderLine.connections(), user.getPosition(), ladderInfo.width());
final boolean isConnectedLeft = checkLeftConnection(ladderLine.connections(), user.getPosition());
decideMoving(isConnectedLeft, isConnectedRight, user);
}
}

private boolean checkRightConnection(final List<Boolean> connections, final int position, final int width) {
if (position == width - 1) {
return false;
}
return connections.get(position);
}

private boolean checkLeftConnection(final List<Boolean> connections, final int position) {
if (position - 1 < 0) {
return false;
}
return connections.get(position - 1);
}

private void decideMoving(final boolean isConnectedLeft, final boolean isConnectedRight, final User user) {
if (!isConnectedLeft && !isConnectedRight) {
return;
}
decideMovingLeftOrRight(isConnectedLeft, user);
}

private void decideMovingLeftOrRight(final boolean isConnectedLeft, final User user) {
if (isConnectedLeft) {
user.setPosition(user.getPosition() - 1);
return;
}
user.setPosition(user.getPosition() + 1);
}
}
10 changes: 10 additions & 0 deletions src/main/java/service/ladder/LadderService.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package service.ladder;

import domain.ladder.Ladder;

public interface LadderService {

void makeLadder(final int width, final int height);

Ladder getLadder();
}
65 changes: 65 additions & 0 deletions src/main/java/service/ladder/LadderServiceImpl.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
package service.ladder;

import domain.ladder.Ladder;
import domain.ladder.LadderInfo;
import domain.ladder.LadderLine;
import domain.ladder.LadderLineConnection;
import repository.ladder.LadderRepository;

import java.util.*;

Choose a reason for hiding this comment

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

자바 컨벤션에서 wild card import 를 지양하라는 문구가 있을텐데요
https://reiphiel.tistory.com/entry/Intellij-java-prevent-wildcard-import
이쪽을 참고해서 적용해주시면 좋을 것 같아요

Copy link
Author

Choose a reason for hiding this comment

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

자동 import여서 차마 이 부분까지는 신경쓰지 못했던 것 같습니다..!! 감사합니다 : )

import java.util.stream.IntStream;

public class LadderServiceImpl implements LadderService {

private final LadderRepository ladderRepository;
private static final String EDGE_OF_LADDER = "|";

public LadderServiceImpl(final LadderRepository ladderRepository) {
this.ladderRepository = ladderRepository;
}

@Override
public void makeLadder(final int width, final int height) {
ladderRepository.setLadder(new Ladder(makeLadderLines(height, width), new LadderInfo(width, height)));
}

private List<LadderLine> makeLadderLines(final int height, final int width) {
return IntStream.range(0, height)
.mapToObj(i -> makeLadderLine(width))
.toList();
}

private LadderLine makeLadderLine(final int width) {
List<Boolean> connections = makeConnections(width);
return new LadderLine(connections, makeLine(connections));
}

private List<Boolean> makeConnections(final int width) {
List<Boolean> connections = new ArrayList<Boolean>();
Random random = new Random();

while (connections.size() < width - 1) {
addConnections(connections, random.nextBoolean());
}

return connections;
}
Comment on lines +37 to +46

Choose a reason for hiding this comment

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

힘드실 것 같긴 하지만 이 부분에 대한 테스트가 있으면 좋을 것 같아요!

대부분 가장 많이 실수하는 경우가 처음 생성 로직이 예상과는 다르게 진행되는 것인데요
이 부분을 테스트를 통해서 보완하는 것이 가장 경험적으로 실수가 적었던 것 같아요

Copy link
Author

Choose a reason for hiding this comment

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

랜덤에 대한 테스트는 필요하다고 생각했었습니다..!! 한 번 테스트 해보도록 하겠습니다 감사합니다 : )


private void addConnections(final List<Boolean> connections, final boolean isConnected) {
if (connections.isEmpty() || !isConnected || !connections.get(connections.size() - 1)) {
connections.add(isConnected);
}
}

private String makeLine(final List<Boolean> connections) {

Choose a reason for hiding this comment

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

이 부분은 view 쪽으로 가면 좋을 것 같아요!
개인적으로는 이런 방향이 가장 좋을 것 같아요

graph TD;
view --순수_데이터만_전달-->controller--로직_처리_위임-->service
service--로직_결과_반환-->controller--순수_데이터_전달-->view
Loading

출력의 결과같은 부분은 서비스에 있는 것보다는 이 관점에서는 view 쪽이 더 적절할 것 같고요

Copy link
Author

Choose a reason for hiding this comment

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

아하..!! 말씀하신 것처럼 출력과 관련된 로직은 view에 위임하는 것이 더 역할분리가 잘 된 것처럼 보이는 것 같습니다!! 감사합니다 : )

final List<String> line = connections.stream()
.map(connection -> LadderLineConnection.of(connection).getLadderConnectionFormat())
.toList();
return String.join("", line) + EDGE_OF_LADDER;
}

@Override
public Ladder getLadder() {
return ladderRepository.getLadder();
}
}
11 changes: 11 additions & 0 deletions src/main/java/service/results/ResultsService.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package service.results;

import domain.results.Results;

import java.util.List;

public interface ResultsService {
void makeResults(final List<String> results);

Results getResults();
}
Loading