Repository navigation
step2 pr합니다. - #135
Merged
Merged
step2 pr합니다.#135
Conversation
javajigi
suggested changes
Jan 9, 2019
javajigi
left a comment
javajigi
left a comment
Contributor
There was a problem hiding this comment.
- Util은 현재 클래스 기반으로 구현하고 대신 인스턴스 메소드를 클래스 메소드로 구현
- ResultView도 그래도 유지. 단 인스턴스 메소드를 클래스 메소드로 구현
- enum을 사용하지 않고 구현해 본다.
코드를 너무 깔끔하게 구현하려고 노력하고 있는데 그 보다는 구현을 빠르게 하는 것에 집중할 것을 추천
아직까지 인스턴스 메소드와 클래스 메소드로 구분해서 구현해야할 부분을 분리하는 역량이 부족함.
굳이 상속을 사용할 필요가 없음에도 불구하고 상속을 사용하는 경우도 있음.
| private List<Integer> axiss = mainControllerUtil.axisNum(); | ||
| private List<Integer> reversalAxis = mainControllerUtil.reversalAxisNum(); | ||
| private List<Point> points = mainControllerUtil.objectCoordinates(inputPoint); | ||
| private int axisSize = mainControllerUtil.getAxisSize(); |
Contributor
There was a problem hiding this comment.
클래스에 인스턴스 변수가 너무 많음.
이 변수들을 모두 main() 메소드에 로컬 변수로 구현한다.
| public static void main(String[] args) { | ||
| coordinatesServes.main(); | ||
|
|
||
| controller.main(); |
Contributor
There was a problem hiding this comment.
controller의 main() 메소드를 이곳에 구현하면 되지 않을까?
MainController가 하는 일이 없음.
|
|
||
| import static org.slf4j.LoggerFactory.getLogger; | ||
|
|
||
| public class MainControllerUtil extends SplitUtil { |
Contributor
There was a problem hiding this comment.
Util 성격의 메소드는 클래스 메소드로 구현한다.
Util 성격의 클래스는 상속 구조로 구현하지 않음.
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| public class SplitUtil { |
Contributor
There was a problem hiding this comment.
메소드가 인스턴스 변수에 접근하지 않으면 굳이 인스턴스 메소드로 구현할 필요 없음
클래스 메소드로 구현한다.
|
|
||
| import java.util.List; | ||
|
|
||
| public class ResultView { |
Contributor
There was a problem hiding this comment.
메소드가 인스턴스 변수에 접근하지 않으면 굳이 인스턴스 메소드로 구현할 필요 없음
클래스 메소드로 구현한다.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
잘한 점 못한 점, 피드백 같이 적어주시면 감사하겠습니다.
step2 진행 중 궁금한 점
MainControllerUtil에 있는 메서드 중 axis에 관련 메서드를 Class로 분리하는게 좋을지
Class를 분리한다면 final 변수 중복이 많아지는데 final변수를 관리하는 Class를 따로 생성하는게 좋은지
ResultView에서 출력 시
for문 안에 if문이 들어가있는 변수들이 있는데 if문을 빼내서 메서드로 생성하는게 좋을지
enum을 사용하려고 잠시동안 Test 해봤는데
enum class 이름 + 변수명 + get변수명
으로 호출할 수 있던데
일반 객체에서 get 하지 않고 메서드를 만들어 호출하는 클래스에서 메서드를 보내는 식으로는
enum 사용이 불가능한지 여부
이 것에 대해서도 알려주시면 감사하겠습니다.