-
Notifications
You must be signed in to change notification settings - Fork 272
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
[지하철 노선도] 임규리 - 리뷰 부탁드리겠습니다! #124
base: main
Are you sure you want to change the base?
Changes from 71 commits
1180179
1b58ada
ed7bcc1
8018b4b
0f98043
a8ef7da
d99bc6a
29ca7cf
953cfd3
5551abe
20cb96d
601330f
314499a
7651fb1
511894e
02346ab
29a85f0
a6715fa
39d2f3b
1bfd40a
189dcee
2f3678c
5476e78
7707323
e2bb5c4
903babe
ae7a66c
9d7524f
442c8de
7a428fb
e998268
2eafd89
4257a0b
c3124af
4491b9a
45a7498
bb8708a
c2d3429
d25207f
5b891cf
a2b1c55
0b6813c
454fe46
1991ff3
a2d7925
f068479
e2a231b
e15d760
ef11df4
bab3160
4f39c4e
0d9c3f8
bfd982b
9249d7e
023b2f8
3a4deac
e7a6fda
8cb85e6
6121177
00adc99
a56dede
9307a52
590fd99
d7a3f24
00f7a13
f890958
c650af2
1d2550e
1aa7842
fb77c33
ca8abcc
7ba0d80
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,17 @@ | ||
package subway; | ||
|
||
import java.util.Scanner; | ||
import subway.config.AppConfig; | ||
import subway.controller.SubwayController; | ||
import subway.view.OutputView; | ||
|
||
public class Application { | ||
public static void main(String[] args) { | ||
final Scanner scanner = new Scanner(System.in); | ||
// TODO: 프로그램 구현 | ||
try { | ||
final AppConfig appConfig = AppConfig.INSTANCE; | ||
final SubwayController subwayController = appConfig.getController(); | ||
subwayController.start(); | ||
} catch (IllegalArgumentException e) { | ||
OutputView.printError(e.getMessage()); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
package subway.config; | ||
|
||
import java.util.Scanner; | ||
import subway.controller.SubwayController; | ||
import subway.service.LineService; | ||
import subway.service.MainService; | ||
import subway.service.SectionService; | ||
import subway.service.StationService; | ||
import subway.view.InputView; | ||
import subway.view.OutputView; | ||
|
||
public enum AppConfig { | ||
|
||
INSTANCE; | ||
|
||
private final Scanner scanner = new Scanner(System.in); | ||
private final InputView inputView = new InputView(scanner); | ||
private final OutputView outputView = new OutputView(); | ||
private final MainService mainService = new MainService(); | ||
private final StationService stationService = new StationService(); | ||
private final LineService lineService = new LineService(); | ||
private final SectionService sectionService = new SectionService(); | ||
private final SubwayController subwayController = new SubwayController(inputView, outputView, mainService, | ||
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. 저도 프리코스 기간동안 코드리뷰를 하며 알게된 사실인데, 함수 매개변수의 개수가 3개가 넘어가면 안좋다고 합니다. 3개 이하로 줄여보는 것은 어떨까요? 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. 해당 코드는 의존성 주입을 위한 부분이라고 생각되는데, DI를 적용하지 않고 클래스 내부에서 의존성을 직접 생성하는 것을 지향하시는걸까요?? 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. 저도 이 부분 의견이 궁금합니다! @kgy1008 |
||
stationService, lineService, sectionService); | ||
|
||
public SubwayController getController() { | ||
return subwayController; | ||
} | ||
} |
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. "Q"와 같은 경우 "QUIT" 이라는 의미를 명확하게 부여했지만, 1이나 2와 같은 경우에 상대적으로 명확하지 않은 것 같습니다. 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,31 @@ | ||
package subway.constants; | ||
|
||
import subway.exception.ErrorMessage; | ||
|
||
public enum MainFeature { | ||
SELECT_ONE("1"), | ||
SELECT_TWO("2"), | ||
SELECT_THREE("3"), | ||
SELECT_FOUR("4"), | ||
QUIT("Q"); | ||
|
||
final String message; | ||
|
||
MainFeature(String message) { | ||
this.message = message; | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return message; | ||
} | ||
Comment on lines
+18
to
+21
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. 저는 getter를 사용하는 것이 메서드 이름을 통해 명확히 "어떤 값(message)을 가져오는지" 드러낼 수 있어 의도에 맞는 함수라고 생각했기 때문에 getter 대신에 toString을 재정의하셔서 사용한 이유가 궁금합니다. 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. 다른 코드에서 이렇게 활용하시는 걸 봐서 따라해 본 것인데, 말씀해주신 부분을 생각해보니 getXXX로 사용하는 편이 의도가 확실할 것 같습니다. |
||
|
||
public static MainFeature getFeatureFromInput(String input) { | ||
for (MainFeature feature : MainFeature.values()) { | ||
if (feature.toString().equalsIgnoreCase(input)) { | ||
return feature; | ||
} | ||
} | ||
throw new IllegalArgumentException(ErrorMessage.INVALID_FEATURE.toString()); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
package subway.constants; | ||
|
||
import subway.exception.ErrorMessage; | ||
|
||
public enum SectionFeature { | ||
SELECT_ONE("1"), | ||
SELECT_TWO("2"), | ||
BACK("B"); | ||
|
||
final String message; | ||
|
||
SectionFeature(String message) { | ||
this.message = message; | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return message; | ||
} | ||
|
||
public static SectionFeature getFeatureFromInput(String input) { | ||
for (SectionFeature feature : SectionFeature.values()) { | ||
if (feature.toString().equalsIgnoreCase(input)) { | ||
return feature; | ||
} | ||
} | ||
throw new IllegalArgumentException(ErrorMessage.INVALID_FEATURE.toString()); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
package subway.constants; | ||
|
||
import subway.exception.ErrorMessage; | ||
|
||
public enum StationLineFeature { | ||
SELECT_ONE("1"), | ||
SELECT_TWO("2"), | ||
SELECT_THREE("3"), | ||
BACK("B"); | ||
|
||
final String message; | ||
|
||
StationLineFeature(String message) { | ||
this.message = message; | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return message; | ||
} | ||
|
||
public static StationLineFeature getFeatureFromInput(String input) { | ||
for (StationLineFeature feature : StationLineFeature.values()) { | ||
if (feature.toString().equalsIgnoreCase(input)) { | ||
return feature; | ||
} | ||
} | ||
throw new IllegalArgumentException(ErrorMessage.INVALID_FEATURE.toString()); | ||
} | ||
} |
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.
enum을 활용하여 controller에 대해 singleton을 보장하는 방식이 인상 깊습니다~!