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

[LBP] 김태우 사다리 1단계 제출합니다 #42

Open
wants to merge 3 commits into
base: twootwoo
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
10 changes: 10 additions & 0 deletions src/main/java/LadderApplication.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import controller.LadderController;

public class LadderApplication {
public static void main(String[] args) {
LadderController ladderController = LadderController.getInstance();

ladderController.run();
}

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

import dto.LineDto;
import model.Ladder;
import model.Line;
import view.LadderOutputView;

import java.util.List;

public class LadderController {
private static final LadderController ladderController = new LadderController();

private final LadderOutputView ladderOutputView = LadderOutputView.getInstance();

private LadderController() {
}

public static LadderController getInstance() {
return ladderController;
}

public void run() {
Ladder ladder = new Ladder(4, 4);
List<Line> lines = ladder.getLines();

ladderOutputView.printResultHeader();
for (Line line : lines) {
LineDto lineDto = LineDto.from(line);
ladderOutputView.printLine(lineDto);
}
}

}
29 changes: 29 additions & 0 deletions src/main/java/dto/LineDto.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package dto;

import model.Line;
import model.Link;
import model.LinkStatus;

import java.util.List;

public class LineDto {
Copy link

Choose a reason for hiding this comment

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

model에서 line에 대해서 알 수 없게 하도록 하기 위해서 해당 dto를 도입한 것으로 예상되요
그치만 제 생각에는 dto를 사용하지 않아도 해결할 수 있는 문제라고 생각하는데 태우님은 어떻게 생각하시나요?
혹시 dto에게 책임을 전가하고 있진 않았나 돌아볼까요? 🤔🤔

private final List<Boolean> linkExistCollection;

private LineDto(List<Boolean> linkExistCollection) {
this.linkExistCollection = linkExistCollection;
}

public static LineDto from(Line line) {
List<Boolean> linkExistCollection = line.getLinks().stream()
.map(Link::getLinkstatus)
.map(LinkStatus::isPresent)
.toList();

return new LineDto(linkExistCollection);
}

public List<Boolean> getLinkExistCollection() {
return List.copyOf(linkExistCollection);
}

}
23 changes: 23 additions & 0 deletions src/main/java/model/Ladder.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package model;

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

public class Ladder {
private final List<Line> lines;

public Ladder(int width, int height) {
List<Line> lines = new ArrayList<>();
for (int i = 0; i < height; i++) {
lines.add(new Line(width));
}

this.lines = Collections.unmodifiableList(lines);
}

public List<Line> getLines() {
return List.copyOf(lines);
}
Comment on lines +16 to +21
Copy link

Choose a reason for hiding this comment

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

리스트의 방어적 복사 활용 좋습니다👍
두 방식에는 어떤 기준을 두고 서로의 반환 방식을 다르게 활용했나요?
태우님의 생각을 알려주세요


Copy link

Choose a reason for hiding this comment

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

불필요한 개행이 있네요 수정해봅시다!🤔

Copy link
Author

@TwooTwoo TwooTwoo Mar 15, 2025

Choose a reason for hiding this comment

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

남겨주신 코맨트를 보고 다시 확인해 보았으나 어떤 부분이 불필요한 개행인지 잘 모르겠습니다..!
혹시 몰라 Google Java Style Guide의 Whitespace 단락을 읽어 보았고, 아래 문구에 의해 제 개행이 문제되지 않는다고 판단했습니다.

A single blank line may also appear anywhere it improves readability, for example between statements to organize the code into logical subsections. A blank line before the first member or initializer, or after the last member or initializer of the class, is neither encouraged nor discouraged.

혹시 제가 놓쳤거나 잘못 알고 있는 부분이 있다면 편하게 지적 부탁드립니다. 감사합니다!

Copy link

Choose a reason for hiding this comment

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

     public List<Line> getLine() {
        return List.copyOf(lines);
    }
 // 여기를 말씀드리는 것이였어요   
}

딱히 잘못되어 보이진 않지만 제가 보기에는 불필요한 개행이라고 생각했어요
태우님의 생각은 어떤가요?

}
99 changes: 99 additions & 0 deletions src/main/java/model/Line.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
package model;

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

import static model.LinkStatus.PRESENT;
import static model.LinkStatus.UNDEFINED;

public class Line {
private static final Random random = new Random();

private final List<Link> links;

public Line(int width) {
int size = width - 1;
List<Link> links = initializeLinks(size);

setupLinks(links);

this.links = List.copyOf(links);
}
Comment on lines +15 to +22
Copy link

Choose a reason for hiding this comment

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

Link, Line이 비대칭적이에요
LineLink의 상태를 직접적으로 변경하고 있어요 -> 객체의 자율성도 깨지고, 무엇보다 캡슐화가 가장 지켜지지 못한 설계라고 볼 수 있죠
LineLink의 내부 동작을 너무 많이 알고 있어요, 결합도가 매우 높아요 (결국 디미터의 법칙 위배)

결합도를 낮추기 위해서 개선해봅시다!


public List<Link> getLinks() {
return List.copyOf(links);
}

private List<Link> initializeLinks(int size) {
List<Link> links = new ArrayList<>();
for (int i = 0; i < size; i++) {
links.add(Link.getUndefinedLink());
}

return List.copyOf(links);
}

private void setupLinks(List<Link> links) {
int index = getRandomStartIndex(links);

while (containsUndefined(links)) {
boolean connectDecider = getConnectDecider();
boolean connectable = isConnectable(index, links);

Link link = links.get(index);
link.setLinkStatus(connectDecider, connectable);

index = getNextIndex(index, links);
}
}

private int getRandomStartIndex(List<Link> links) {
return random.nextInt(links.size());
}
Comment on lines +51 to +53
Copy link

Choose a reason for hiding this comment

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

현재 이 구도는 테스트가 거의 불가능해보이는 설계로 보여요
랜덤을 통한 비결정적인 로직은 테스트의 불가능성을 유발시키는데
어떻게 개선 가능할까요? (테스트는 항상 꼭 작성해주셔야합니다, 그래야 제가 정상적으로 동작하는지 확인할 수 있어요)


private boolean containsUndefined(List<Link> links) {
return links.stream()
.anyMatch(link -> link.getLinkstatus() == UNDEFINED);
}

private boolean getConnectDecider() {
return random.nextBoolean();
}

private boolean isConnectable(int index, List<Link> links) {
if (isFirstIndex(index)) {
return isRightNotPresent(index, links);
}
if (isLastIndex(index, links)) {
return isLeftNotPresent(index, links);
}

return isRightNotPresent(index, links) && isLeftNotPresent(index, links);
}

private boolean isFirstIndex(int index) {
return index == 0;
}

private boolean isLastIndex(int index, List<Link> links) {
return index == links.size() - 1;
}

private boolean isRightNotPresent(int index, List<Link> links) {
Link rightLink = links.get(index + 1);

return rightLink.getLinkstatus() != PRESENT;
}

private boolean isLeftNotPresent(int index, List<Link> links) {
Link leftLink = links.get(index - 1);

return leftLink.getLinkstatus() != PRESENT;
}

private int getNextIndex(int index, List<Link> linkStatuses) {
return ++index % linkStatuses.size();
}

}
39 changes: 39 additions & 0 deletions src/main/java/model/Link.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package model;

public class Link {
private LinkStatus linkstatus;

private Link(LinkStatus linkstatus) {
this.linkstatus = linkstatus;
}

public static Link getUndefinedLink() {
return new Link(LinkStatus.UNDEFINED);
}

public void setLinkStatus(boolean connectDecider, boolean connectable) {
validateUpdateStatus();

if (connectDecider && connectable) {
this.linkstatus = LinkStatus.PRESENT;
return;
}

this.linkstatus = LinkStatus.ABSENT;
}
Comment on lines +14 to +23
Copy link

Choose a reason for hiding this comment

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

Link 객체는 스스로 자신의 상태를 관리하는 게 아니라 외부(Line)에서의 상태에 의존하여 내부 상태를 변경하고 있어요
Side Effect의 발생 가능성을 가지고 있어요! 어떻게 개선할 수 있을까요? 🤔🤔


public LinkStatus getLinkstatus() {
return linkstatus;
}

private void validateUpdateStatus() {
if (isAlreadyFixed()) {
throw new UnsupportedOperationException("이미 값이 결정된 링크입니다");
}
}

private boolean isAlreadyFixed() {
return this.linkstatus != LinkStatus.UNDEFINED;
}

}
10 changes: 10 additions & 0 deletions src/main/java/model/LinkStatus.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package model;

public enum LinkStatus {
UNDEFINED, ABSENT, PRESENT;

public boolean isPresent() {
return this == PRESENT;
}

}
47 changes: 47 additions & 0 deletions src/main/java/view/LadderOutputView.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package view;

import dto.LineDto;

public class LadderOutputView {

private static final String INDENTATION = " ";
private static final String DASH_COUPLER = "-----";
private static final String BLANK_COUPLER = " ";
private static final String PILLAR = "|";

private LadderOutputView() {
}

private static final LadderOutputView ladderOutputView = new LadderOutputView();

public static LadderOutputView getInstance() {
return ladderOutputView;
}
Comment on lines +12 to +19
Copy link

Choose a reason for hiding this comment

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

싱글톤 패턴을 적용하셨군요

  • 싱글톤 패턴을 도입한 태우님의 생각이 궁금해요!
  • 지연 초기화라는 학습 키워드를 통해서 구현된 싱글톤 패턴을 더 개선해봅시다! (개선하고 저한테 설명도 해주시면 좋을 것 같아요)


public void printResultHeader() {
System.out.println("실행결과");
System.out.println();
}

public void printLine(LineDto lineDto) {
StringBuilder output = new StringBuilder()
.append(INDENTATION)
.append(PILLAR);

for (boolean isExist : lineDto.getLinkExistCollection()) {
String coupler = getCoupler(isExist);
output.append(coupler)
.append(PILLAR);
}

System.out.println(output);
}

private String getCoupler(boolean isLinkExist) {
if (isLinkExist) {
return DASH_COUPLER;
}
return BLANK_COUPLER;
}

}