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

[201700981 신성희]1주차 미션을 제출합니다. #8

Open
wants to merge 29 commits into
base: main
Choose a base branch
from

Conversation

Shsin9797
Copy link

실행하면 오류뜨네요 ..

@Shsin9797
Copy link
Author

image
이오류가 떠서 잘 돌아가는지 확인을 못해봤네요...

@shin-mallang
Copy link
Contributor

확인해보겠습니다~

@shin-mallang
Copy link
Contributor

@Shsin9797
클래스 코드를 복사해서 어딘가에 붙여넣기 이후,
기존 Application 클래스를 제거하고 다시 만들어주시면 해결됩니다~

Copy link
Contributor

@shin-mallang shin-mallang left a comment

Choose a reason for hiding this comment

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

안녕하세요 성희님~
예~~~~~~ 전에 플러터 강의때 같이 프로젝트 진행했던 걸로 기억하는데 맞나요? ㅎㅎ

1단계 미션 고생하셨습니다~
그런데 코드를 실행해보니 아직 제대로 작동은 하지 않는 것 같아요..ㅠㅠ
스크린샷 2024-02-18 오전 12 32 05

해당 부분포함하여 테스트가 모두 정상 작동하도록 고쳐주시면 감사하겠습니다~

전체적으로 드리고 싶은 말씀은 여기에 적을게요!

컨벤션을 어느정도 잘 지켜주셔서 너무 좋았습니다.
그런데 메서드 순서는 아직 컨벤션에 맞게 작성되지 않은 것 같아요~
성희님이 작성된 코드를 다른 사람이 읽을 때 메서드 순서가 어떤지에 따라서도 코드 파악이 쉬워질수도, 어려워질 수도 있습니다~
제공해드린 컨벤션 문서를 참고해서 메서드 순서를 재배치해 주시면 감사하겠습니다 :)


커밋을 정말 잘 분리해 주셨어요~ 💯
다만 한가지 아쉬운 점은 메세지를 조금만 더 명확하게 해주시면 어떨까 하는 아쉬움이 있습니다 ㅎㅎ
다음 미션에서는 커밋 메세지를 좀 더 구체적으로 적어주시면 좋을 것 같아요!


코드에 주석이 너무 많은 것 같습니다~
주석을 무조건 사용하지 말라는 것은 아니지만, 주석 역시도 유지보수의 대상이므로
주석을 많이 사용하게 되면 그만큼 코드 변경 시 신경써야할 점들이 많아지는 것을 의미해요~

주석이 없더라도 코드를 쉽게 이해할 수 있게 작성하도록 노력해봐요 :)


마지막으로는 클래스 분리에 관한 내용입니다~

지금은 하나의 Application 코드에 모든 로직을 작성해 주셨어요!
이번 미션은 간단하기 떄문에 이렇게 작성해도 문제는 없지만,
구현해야하는 기능이 복잡해 질 수록 이렇게 하나의 클래스에 모든 기능을 구현하게 되면 유지보수에 어려움을 겪을 수 있습니다~
클래스를 조금 나누어 책임을 분리하여, 가독성과 유지보수성을 올릴 수 있게 코드를 변경해보셨으면 좋겠습니다.


리뷰에 대해서, 또는 미션 진행하면서 궁금한 점이 있으시다면 언제든 편하게 질문 주세요~
리뷰 반영이 끝났으면 제게 말씀해 주세요 :)

Comment on lines 23 to 27
}

//메서드 선언

public static boolean checkInputNum(String a,int start,int end) throws IllegalArgumentException{
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
}
//메서드 선언
public static boolean checkInputNum(String a,int start,int end) throws IllegalArgumentException{
}
//메서드 선언
public static boolean checkInputNum(String a,int start,int end) throws IllegalArgumentException{

공백 한 칸 없애주세요~

boolean doGame = true;

while (doGame) {
int[] userNum = getThreeNums();
Copy link
Contributor

Choose a reason for hiding this comment

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

배열 대신 List를 사용해볼까요?


//메서드 선언

public static boolean checkInputNum(String a,int start,int end) throws IllegalArgumentException{
Copy link
Contributor

Choose a reason for hiding this comment

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

String a 가 의미하는 게 뭘까요?
의미를 좀 더 분명하게 바꿔볼까요?

public static boolean checkInputNum(String a,int start,int end) throws IllegalArgumentException{

try {
int b = Integer.parseInt(a) ;
Copy link
Contributor

Choose a reason for hiding this comment

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

마찬가지로, a, b로만 작성되어 있으면 코드의 의미를 파악하기 힘들어집니다~
코드의 가독성을 높여볼까요?

<br>
---

## 🎱 기능 목록
Copy link
Contributor

Choose a reason for hiding this comment

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

기능 목록 잘 작성해 주셨네요 💯

@Shsin9797
Copy link
Author

@Shsin9797 클래스 코드를 복사해서 어딘가에 붙여넣기 이후, 기존 Application 클래스를 제거하고 다시 만들어주시면 해결됩니다~

말씀해주신대로 했는데 다른 문제가 생겼습니다 ㅠㅠ
어떤게 문제인지 잘 모르겠습니다 ㅜㅜ .
image

@Shsin9797
Copy link
Author

이번과제는 여기까지 하고 클래스 구현은 다음에 해보겠습니다 ㅠㅠ

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants