search

Elegie0 / 42Seoul

0 stars 0 forks source link

코드리뷰(eunhkim) #1

Open eunhyulkim opened 4 years ago

eunhyulkim commented 4 years ago

안녕하세요. 수고 많으셨습니다! 직접 코드를 클론해서 테스트해보았고, 아래와 같이 리뷰 남깁니다.

에러

특별한 에러는 보이지 않았습니다. 제대로 답을 내고, 예외처리가 꼼꼼하게 되어 있습니다.

개선사항

  1. 예외처리시 아래 케이스의 에러 메시지에 오타가 있는 것 같습니다. 
    case ERR_TOO_MANY_ARGUMENTS
"Too mant arguments. " -> many
    case ERR_WRONG_NUM
"Argument must be larger than 2" --> 1
  2. Readable한 코드가 이번 테스트의 주요 평가 관점인데, 제 역량이 부족해서 새로운 자료형을 만들거나 INIT_MAX를 처리하는 코드가 쉽게 읽히지는 않았던 것 같습니다.
  3. 그 밖에 문제는 아니겠지만 소스 파일이 2개 쓰였고, 25줄 넘는 함수가 1개 있어서 개인적으로 아쉬웠습니다. 제약 안에서 좋은 결과를 만드는, 더 42스러웠으면 좋겠다는 느낌이 들었습니다.

배운 점

전처리 부분에서 많이 배울 수 있었습니다. ft_memset을 이용해서 최대값을 만드는 지점이 인상 깊었고, 특히 시스템의 차이를 고려했다는 점은 제가 놓쳤던 부분이기에 와닿았던 것 같습니다. 고수의 느낌이 물씬 나서 헤더 파일은 저장해두고 더 배우고 싶어요. 다시 한 번 정말 수고 많으셨습니다!

Elegie0 commented 4 years ago

오타는 말씀 안해주셨으면 몰랐을 것 같아요ㅋㅋ

최댓값 만드는 부분은, 원래는 매크로를 안쓰고 만들었는데, -wall -wextra -werror 옵션을 넣었더니 부호없는 정수형이 0보다 큰지 작은지 판단할 때 에러가 발생하더라구요. 수정하려고 부득이하게 매크로를 써서 가독성이 떨어지는 부분 인정합니다ㅠㅠ

나름대로 놂을 맞춘다고 맞췄는데 26줄이 있었군요...ㅠㅠ 발견해주셔서 감사합니다!

리뷰 감사합니다~!!