학생 관리 프로그램 - 이태훈 #7
Conversation
| import java.util.Scanner; | ||
|
|
||
| public class ManagementController { | ||
| ManagementService managementService; |
There was a problem hiding this comment.
이 부분은 private 접근 제어자로 설정해서 외부에서 접근을 하지 않도록 설정하는 것이 일반적인 것으로 아는데 default 접근제어자로 설정하신 이유가 따로 있으실까요?
There was a problem hiding this comment.
앗 이건 제가 빠뜨리고 private를 쓰지 않은것 같네요..!ㅜㅜ
| String cmd = InputView.readMenuBarCmd(scan); | ||
| Validate.isValidCmd(cmd); | ||
| switch (cmd) { | ||
| case "1"://학생 등록 |
There was a problem hiding this comment.
지금처럼 주석을 사용하여 switch-case에서 각 case의 용도를 파악하는 것도 좋지만 이전에 사용하셨던 Constant 클래스를 사용하거나, Enum을 사용하는 것도 가독성 측면에서 좋은 방식이라고 생각합니다! 어떻게 생각하시나요??
There was a problem hiding this comment.
상수처리하는 것을 깜빡한것 같아요..! 근데 Enum을 쓰는 방법은 생각치 못하고 있었는데 괜찮은 방법일 것 같아요!!
| } | ||
|
|
||
| public void setAverage() { | ||
| this.average = (double)(koreanGrade + englishGrade + mathGrade) / GradeConstant.SUBJECT_CONUT; |
There was a problem hiding this comment.
현재는 setAverage 메서드가 빈번하게 호출되네요! 보다 호출을 줄이기 위한 방법을 고민해보셔도 좋을 것 같습니다!!
ex) average 멤버가 참조되기 직전에만 setAverage를 호출한다던가..
There was a problem hiding this comment.
아 average가 참조되기 전에 사용하면 되겠군요!! 좋은 피드백 감사합니다!
| studentRepository.save(student); | ||
| } | ||
|
|
||
| public StudentDTO findStudent(Scanner scan){ |
There was a problem hiding this comment.
각 비즈니스 로직을 실행하기 위해 매번 Scanner 객체를 인자로 주고 있는데 이렇게 처리하신 의도가 궁금합니다!
There was a problem hiding this comment.
Scanner라는 입력을 받는 객체를 어디에서 선언해야할지 고민하다가 View와 domain을 연결해주는 Controller부분에서 선언하는것이 맞다고 생각해서 그렇게 하였고, 이 생각으로 인해 다른 계층에서 따로 Scanner를 선언하지 않고 controller에서 인자로 주게 코딩한것 같습니다. 안그래도 이것때문에 고민을 많이 했는데 Scanner는 계층에 상관없이 선언해도 괜찮을까요??
|
|
||
| public class StudentsRepository { | ||
|
|
||
| public static final Map<Integer, Student> students = new ConcurrentHashMap<>(); |
There was a problem hiding this comment.
ConcurrentHashMap을 여기서 보게 되네요! 태훈님의 지식의 깊이를 알 수 있는 부분이네요 :)
내가 개발한 기능
내가 개발할 때 유의 깊게 개발한 부분
내가 개발하면서 들었던 의문 사항
기능 면에서 부족했다고 생각되는 부분