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

[STEP 1] Gray, Hamzzi #203

Open
wants to merge 11 commits into
base: ic_11_hamzzi
Choose a base branch
from

Conversation

kkomgi
Copy link

@kkomgi kkomgi commented Jan 4, 2024

완료된 작업 (STEP1)

리뷰어 : @DevWooHyeon

고민되었던 점

  • varlet의 사용 시기를 결정.
  • 들여쓰기를 최대 2단계로 제한하고 필요한 경우 코드를 간추리는 방법 도입.
  • 중복이 제거된 무작위 숫자 생성 방법 구현.
  • git merge 시 충돌 관리.
  • 플로우 차트 작성 시 적절한 도형 선택.

해결되지 않은 문제

  • 커밋을 완료한 후 변경된 코드를 병합할 때 시각적으로 변경 내용을 확인하고 코드 간의 차이를 판단하는 데 어려움이 있음.

💌 완료된 작업에 대한 피드백이 있다면 알려주세요 :)

Copy link

@Developer-Nova Developer-Nova left a comment

Choose a reason for hiding this comment

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

안녕하세요 햄찌, 그레이 첫 프로젝트의 첫 PR 고생 많으셨습니다.
아카데미를 시작하는 첫 프로젝트를 함께 하게 되어서 기쁩니다!😃
제가 말씀드리는 리뷰는 절대적인 정답이라고 할 수는 없지만 다시 한번 생각하면서 공부하시면 많은 도움이 될거라고 생각합니다!!👊👊

칭찬드리고 싶은점

  • 함수의 기능분리에 신경을 많이 써주신것 같습니다!
  • 커밋은 미래의 본인을 위한 기록이라 생각합니다. 히스토리가 잘 보일 수 있게 로직 구현 별 커밋을 습관화하시길 바랍니다💪
  • swift 문법에 대한 이해도가 높은것 같습니다!

고민되었던 점에 대해서

  • varlet 의 사용 시기
    • 잘 구분해서 사용해주신것 같습니다. 두개의 차이점에 대해서는 잘 알고 계실거라고 생각됩니다.
  • 들여쓰기를 최대 2단계로 제한하고 필요한 경우 코드를 간추리는 방법 도입.
    • 코드의 최적화에 대해서 고민하신것 같습니다.
    • 최적화도 중요하고 당연히 깊은 고민을 해야하지만 지금은 컨벤션과 네이밍 그리고 함수의 기능분리에 대해서 조금더 고민해 보는게 좋을것같습니다!!
  • 중복이 제거된 무작위 숫자 생성 방법 구현.
    • Set을 활용해서 잘 구현해주신것같습니다👍

해결되지 않은 문제

  • 짝 프로그래밍을 진행하면서 변경된 코드를 병합할일이 없다고 생각 됩니다
  • Git 을 공부하는 것도 중요하지만 서로의 코드를 명확히 이해하고 설명할 수 있는게 지금은 더 중요하다고 생각됩니다!

@@ -6,5 +6,59 @@

import Foundation

Choose a reason for hiding this comment

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

import Foundation 을 사용하신 이유가 궁금합니다!!

Foundation 이 어떤 역할을 하는지 설명해주세요!

@@ -6,5 +6,59 @@

import Foundation

print("Hello, World!")
var comNumbers = [Int]()

Choose a reason for hiding this comment

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

변수의 네이밍은 줄임말보다는 명확한게 더 좋을거같습니다!


func generateRandomNumbers() -> [Int] {
var setNumber = Set<Int>()

Choose a reason for hiding this comment

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

swift 에 이미 정의 되어있는 키워드는 변수명에 사용하지 않는 것이 좋을것같습니다!

return [Int](setNumber)
}

func judgeResult(val: [Int]) -> (countStrike: Int, countBall: Int) {

Choose a reason for hiding this comment

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

judge 라는 동사보다는 judgment 라는 명사를 사용하는것이 좋을것 같습니다!

파라미터의 네이밍또한 적절하지 않은것 같습니다!

Comment on lines +28 to +34
for i in 0...2 {
if (comNumbers.contains(userNumbers[i]) && comNumbers[i] == userNumbers[i]) {
countStrike += 1
} else if (comNumbers.contains(userNumbers[i])) {
countBall += 1
}
}

Choose a reason for hiding this comment

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

if 문에서 소괄호는 생략가능한데 생략하지 않은 이유가 궁금합니다!

Comment on lines +37 to +38
return (countStrike, countBall)
}

Choose a reason for hiding this comment

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

튜플을 적절하게 잘 사용해주신것같습니다👍👍

Comment on lines +40 to +44
func startGame() {
comNumbers = [Int]()
userNumbers = [Int]()
attemp = 9
comNumbers = generateRandomNumbers()

Choose a reason for hiding this comment

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

comNumbers 를 빈배열로 초기화하고 랜덤 숫자를 다시 할당하신 이유가 있을까요??
빈배열로 초기화 하는 코드는 불필요하다고 생각됩니다!!
userNumbers 또한 while 문에서 랜덤 숫자를 할당하고 있기 때문에 불필요하다고 생각됩니다!

Comment on lines +47 to +60
while attemp > 0 {
userNumbers = generateRandomNumbers()
print("임의의 수 : \(userNumbers)")

let result = judgeResult(val: userNumbers)

if result.countStrike == 3 {
print("WIN!")

return
}
attemp -= 1
print("남은 기회 : \(attemp)")
}

Choose a reason for hiding this comment

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

while 문을 적절히 잘 사용하신것같습니다!😀

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.

3 participants