-
Notifications
You must be signed in to change notification settings - Fork 37
[LBP] 윤준석 사다리 1단계 제출합니다. #39
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
base: junseok0304
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
import model.Ladder; | ||
import utils.ExceptionHandler; | ||
import view.LadderView; | ||
|
||
public class Application { | ||
public static void main(String[] args) { | ||
try { | ||
Ladder ladder = Ladder.of(4, 4); | ||
LadderView.printLadder(ladder); | ||
} catch (Exception e) { | ||
ExceptionHandler.handleException(e); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
package model; | ||
|
||
public class Ladder { | ||
private final Lines lines; | ||
|
||
public Ladder(Lines lines) { | ||
this.lines = lines; | ||
} | ||
|
||
public static Ladder of(int width, int height) { | ||
return new Ladder(LadderGenerator.generate(width, height)); | ||
} | ||
|
||
public Lines getLines() { | ||
return lines; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
package model; | ||
|
||
public class LadderGenerator { | ||
public static Lines generate(int width, int height) { | ||
return LineGenerator.generate(width, height); | ||
} | ||
} | ||
Comment on lines
+3
to
+7
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 맞습니다,, 래더를 반환할 것 같지만 라인스를 반환 하므로 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
package model; | ||
|
||
import java.util.*; | ||
import java.util.stream.*; | ||
|
||
public class LadderValidator { | ||
private static final Random RANDOM = new Random(); | ||
|
||
public static void validate(List<Line> lines, int width) { | ||
IntStream.rangeClosed(0, width) | ||
.forEach(i -> validateColumn(lines, i, width)); | ||
} | ||
|
||
private static void validateColumn(List<Line> lines, int col, int width) { | ||
boolean emptyColumn = lines.stream().noneMatch(line -> hasBridgeAt(line, col, width)); | ||
if (emptyColumn) { | ||
connect(lines.get(RANDOM.nextInt(lines.size())), col, width); | ||
} | ||
} | ||
|
||
private static boolean hasBridgeAt(Line line, int col, int width) { | ||
if (col == 0) return line.hasBridgeAt(col); | ||
if (col == width) return line.hasBridgeAt(col - 1); | ||
return line.hasBridgeAt(col - 1) || line.hasBridgeAt(col); | ||
} | ||
|
||
|
||
private static void connect(Line line, int col, int width) { | ||
if (col == width) line.setBridgeAt(col - 1); | ||
else line.setBridgeAt(col); | ||
} | ||
|
||
} | ||
Comment on lines
+31
to
+33
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 클래스 내의 마지막 멤버와 클래스를 닫는 괄호 사이에 줄바꿈이 있을 때도 있고 없을 때도 있는 것 같아요. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 해당 부분을 미처 확인하지 못한 것 같습니다! |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
package model; | ||
|
||
import java.util.*; | ||
|
||
public class Line { | ||
private final List<Boolean> points; | ||
|
||
public Line(List<Boolean> points) { | ||
this.points = points; | ||
} | ||
|
||
public List<Boolean> getPoints() { | ||
return points; | ||
} | ||
Comment on lines
+12
to
+14
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. points 참조를 그대로 반환한다면, 외부에서 특정 지점의 정보를 변경할 수 있을 것 같아요. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 리스트의 복사본을 사용해서 방지할 수 있겠습니다! public List getPoints() { There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 좋습니다! 이제 외부로 반환된 리스트가 변경되더라도 Line의 원본 데이터에는 변경이 전파되지 않겠네요! 그렇다면 조금 더 나아가서 아래와 같은 경우가 생긴다해도 괜찮을까요? List<Boolean> points = line.getPoints(); // 복사본을 반환
// 어떤 개발자가 반환된 points를 변경함
points.add(true); 이때도 points가 line이 반환하려는 값을 나타낸다고 할 수 있을까요? 변경 전파를 막는 것과 함께 불변까지 가져간다면 더욱 안전한 데이터 전달이 될 것 같은데 어떻게 생각하시나요?
|
||
|
||
public boolean hasBridgeAt(int index) { | ||
return points.get(index); | ||
} | ||
|
||
public void setBridgeAt(int index) { | ||
points.set(index, true); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
package model; | ||
|
||
import java.util.*; | ||
|
||
public class LineGenerator { | ||
|
||
public static Lines generate(int width, int height) { | ||
List<Set<Integer>> reserved = ReservedPositionGenerator.generate(width - 1, height); | ||
List<Line> lines = new ArrayList<>(); | ||
|
||
for (int row = 0; row < height; row++) { | ||
Line previous = getPreviousLine(lines, row); | ||
Line line = SingleLineGenerator.generate(width - 1, reserved.get(row), previous); | ||
lines.add(line); | ||
} | ||
|
||
LadderValidator.validate(lines, width - 1); | ||
return new Lines(lines); | ||
} | ||
|
||
private static Line getPreviousLine(List<Line> lines, int row) { | ||
if (row == 0) { | ||
return null; | ||
} | ||
return lines.get(row - 1); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
package model; | ||
|
||
import java.util.List; | ||
|
||
public class Lines { | ||
private final List<Line> lines; | ||
|
||
public Lines(List<Line> lines) { | ||
this.lines = lines; | ||
} | ||
|
||
public List<Line> getLines() { | ||
return lines; | ||
} | ||
} | ||
Comment on lines
+5
to
+15
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
일급 컬렉션이라고 말씀하셨는데, 준석님이 생각하시는 일급 컬렉션이란 무엇인가요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 지적해주신대로, 일급 컬렉션의 특징을 가지지 못하고 있는 것 같습니다. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
package model; | ||
|
||
import java.util.*; | ||
import java.util.stream.*; | ||
|
||
public class ReservedPositionGenerator { | ||
private static final Random RANDOM = new Random(); | ||
|
||
public static List<Set<Integer>> generate(int width, int height) { | ||
List<Set<Integer>> reserved = IntStream.range(0, height) | ||
.mapToObj(i -> new HashSet<Integer>()) | ||
.collect(Collectors.toList()); | ||
|
||
IntStream.range(0, width).forEach(i -> | ||
reserved.get(RANDOM.nextInt(height)).add(i)); | ||
|
||
return reserved; | ||
} | ||
Comment on lines
+9
to
+18
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IntStream을 사용하신 특별한 이유가 있으신가요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 가독성을 위해서 사용했던 것 같습니다. for 루프를 이용해도 문제없을 것 같습니다. |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
package model; | ||
|
||
import java.util.*; | ||
import java.util.stream.*; | ||
|
||
public class SingleLineGenerator { | ||
private static final Random RANDOM = new Random(); | ||
|
||
public static Line generate(int width, Set<Integer> reserved, Line prev) { | ||
List<Boolean> points = new ArrayList<>(Collections.nCopies(width, false)); | ||
|
||
reserved.forEach(i -> { | ||
if (isNotOverlap(points, prev, i)) { | ||
points.set(i, true); | ||
} | ||
}); | ||
|
||
IntStream.range(0, width).forEach(i -> { | ||
if (!points.get(i) && isNotOverlap(points, prev, i)) { | ||
points.set(i, RANDOM.nextBoolean()); | ||
} | ||
}); | ||
Comment on lines
+12
to
+22
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 현재 코드에서는 Stream의 forEach() 내부에서 리스트(points)의 상태를 변경하고 있어요. forEach() 내부에서 아래 공식 문서에서도 스트림 내 부작용(Side Effects)은 피해야 한다고 명시되어 있어요 😄 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 추가로 어떤 작업을 하는지 이해하기 어려웠습니다. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 스트림의 사용 원칙을 위배한 코드가 되었군요 ㅜㅜ |
||
|
||
ensureOneBridge(points, prev); | ||
return new Line(points); | ||
} | ||
|
||
private static boolean isNotOverlap(List<Boolean> points, Line prev, int i) { | ||
if (prev != null && prev.hasBridgeAt(i)) return false; | ||
if (i > 0 && points.get(i - 1)) return false; | ||
return true; | ||
} | ||
Comment on lines
+28
to
+32
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 라인 수가 1줄이라서 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 그렇습니다! { }를 사용하지 않았을 때의 side effect가 있을까요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 성능 측면에선 없겠지만, 읽을 때 일관성이 떨어진다는 느낌을 받았어요. 저는 꼭 |
||
|
||
private static void ensureOneBridge(List<Boolean> points, Line prev) { | ||
if (points.contains(true)) return; | ||
|
||
IntStream.range(0, points.size()) | ||
.filter(i -> canSetBridge(points, prev, i)) | ||
.findFirst() | ||
.ifPresent(i -> points.set(i, true)); | ||
} | ||
|
||
private static boolean canSetBridge(List<Boolean> points, Line prev, int index) { | ||
return (prev == null || !prev.hasBridgeAt(index)) | ||
&& (index == 0 || !points.get(index - 1)); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
package utils; | ||
|
||
import org.slf4j.*; | ||
|
||
public class ExceptionHandler { | ||
private static final Logger logger = LoggerFactory.getLogger(ExceptionHandler.class); | ||
|
||
public static void handleException(Exception e) { | ||
System.err.println("시스템 오류 : " + e.getMessage()); | ||
logger.error("시스템 오류 : ", e); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
package view; | ||
|
||
import model.*; | ||
|
||
public class LadderView { | ||
private static final String BRIDGE = "-----|"; | ||
private static final String SPACE = " |"; | ||
private static final String BAR = "|"; | ||
|
||
public static void printLadder(Ladder ladder) { | ||
printLines(ladder.getLines()); | ||
} | ||
|
||
private static void printLines(Lines lines) { | ||
lines.getLines().forEach(LadderView::printLine); | ||
} | ||
|
||
private static void printLine(Line line) { | ||
System.out.print(BAR); | ||
line.getPoints().forEach(LadderView::printPoint); | ||
System.out.println(); | ||
} | ||
|
||
private static void printPoint(Boolean point) { | ||
if (point) { | ||
System.out.print(BRIDGE); | ||
return; | ||
} | ||
System.out.print(SPACE); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ladder는 생성자로도 생성이 가능하고 정적 팩터리 메서드
of()
로도 생성이 가능한 것이 의도된 것인가요?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
사다리 생성과 커스텀 생성이 둘 다 가능한 의도된 로직이나 아직 제대로 활용은 못한 것 같습니다!