sulum
119
2021-01-04 23:13:47 작성 2021-01-04 23:37:09 수정됨
28
1821

코드리뷰가 원래 이런건가요?



그동안 SM만 하다가 코드리뷰가 있는 회사에 처음 왔습니다. 


 지금까지 4~5번 진행했는데


 인수인계도 받지 않은 상태에서 소스코드만 보고 익힌 업무 지식 가지고


구현한 프로그램을 리뷰 받았습니다. 특정 페이지의 사용자 로그를 남기는 프로그램 이었는데요...


하나의 Model을 가지고 각 업무적 행위가 행해지는곳에 코드를 삽입하고 이력을 남기도록 했는데요...


한숨을 푹푹 쉬면서 왜 곳곳에 코드를 이렇게 많이 써놨냐,


6개월이 지났는데 기본적인 로직도 모르느냐 ....매번 1시간 넘게 이러니까


사람들 앞에서 공개망신을 당하는것 같습니다. 지칩니다...


저희 회사 언어는 php를 쓰는데요.


이력정보를 팝업으로 보여주기 위해 백엔드에서 코드값을 이름으로 바꿔주기 위해


함수 array_walk를 이용해 익명함수로 배열 요소를 돌면서


코드->이름으로 바꿔주는 작업을 하고 화면단에 던져줘서 display되게 했는데


왜 화면단에서 한번 Loop 돌릴걸 백엔드에서 한번 돌고 던지느냐 비난조로 지적하고...


맞는말인건 알겠는데 비난하는 어조로 계속 들으니까 점점 일하기가 싫고


코드리뷰가 원래 여러사람 앞에서 공개 비난하는것인가 라는 생각이 듭니다.


  답답해서 하소연해봤습니다. ㅠ






0
  • 댓글 28

  • Frudy
    6k
    2021-01-04 23:31:56
    코드리뷰 없는회사다니는 제입장에서는,
    정말로 비난조로 개무시하며 코드리뷰한다면,

    얼굴에 철판깔고
    1. 제 코드의 잘못된부분이 어디고,
    2. 그 코드가 잘못된 이유와 그 근거는 무엇이며,
    3. 어떻게 개선할 수 있는지 여쭤보겠습니다.

    그걸로 실력쌓고 어떻게든 더 좋은곳으로 이직하려고
    속으로 칼을 갈겠습니다.

    비난당하는동안은 힘들겠지만,
    그사람들하고 평생 일할거 아니잖아요?

    작성자님은 더 잘할수있는 분이에요.
    비난하는 사람보다 더 잘해지셔서 꼭 때려치고 더좋은곳 가세요.

    근데 그 스트레스수준이 너무 높여서 견디기힘들겠으면
    어쩔수없이 나와야할지도몰라요.
    살려고일하는거지 정신병 얻으려고 일하는거 아니니까요.

    아무쪼록 앞으로 하시는일 잘되시길 바라겠습니다.
    더불어 저도 인생좀 폈으면ㅜㅜㅜㅜ
  • sulum
    119
    2021-01-04 23:36:37

    Frudy님 하소연 들어주셔서 감사합니다 ㅠㅠ

  • Frudy
    6k
    2021-01-04 23:43:40 작성 2021-01-04 23:44:18 수정됨

    화면단에서 한번 Loop 돌릴걸 백엔드에서 한번 돌고 던지느냐 비난조로

    ㅡㅡ


    자세한상황을 모르지만 이건궁금하네요.

    어떤로직을 실행하길래

    화면단에서 도는거랑

    서버에서 도는거가지고 비난할까요?


    저라면 해당로직이 서버말고 화면단에서 돌면

    무슨장점이 있고,

    서버에서 돌면 무슨단점이 있는지 물어볼거에요.


    이에대해 합리적인 근거를 제시하지 못한다면,

    그냥 지랑 다른방법으로 구현한게 싫은거 아니에요?

  • sulum
    119
    2021-01-04 23:51:45 작성 2021-01-04 23:58:53 수정됨

    Frudy


    사용자가 행한 행위 ex) 입금 유형 변경, 입금 구분 변경, 대납처리 완료...등등을 코드화 해서 이력테이블에 저장합니다. Key값은 정산 번호로요.


    사용자 행위에 대한 코드값은 아래와 같이 미리 정의 해놓습니다.

    70 : 입금 유형 변경

    60 : 입금 구분 변경 등등...


    이력 테이블 정의는 아래와 같습니다

    이력번호(PK), 정산번호(FK), 처리자 고유번호, 사용자 행위 코드, 등록일시 


    이 이력 정보를 db에서 조회해서  화면단 넘겨주기 전에

    PHP에서 제공하는

    array_walk(다차원 배열 요소를 순회하면서 각 요소에 변경가능하게 해주는 함수)라는

    빌트인 함수를 이용해 코드를 '이름'으로 변경해주고 화면에 던진 후 그대로 뿌려주게 했습니다.


    업무 성격상 정산번호 1개에 이력이 많아봐야 2~10개 내외라서 상관없을거라고 생각했습니다.


    근데 지적받기를 (코드 : 이름) 매핑 정보를 화면으로 같이 넘겨서 한번에 loop돌면서 뿌려주지

    왜 Backend 에서 미리  loop를 돌아서 변경해준 후 넘겨줘서 비효율적으로 동작하게 하냐 였습니다.




     

      
  • Frudy
    6k
    2021-01-04 23:57:32
    음..... 여전히 잘 이해가 안되는부분이있네요,

    코드를 이름으로 바꾸는걸
    왜 서버에서하면 비효율적인일이 되고
    왜 화면단에서 하면 효율적인일이 되는지 모르겠네요.

    제가부족해서 이해를못하는것이니 넘어가세요 ㅜㅜ
  • sulum
    119
    2021-01-04 23:59:56

    Frudy 


    코드리뷰에서 지적받은 요지는 왜 loop를 2번 돌게 하냐 였습니다. 관심가져주셔서 감사합니다. ㅠ

  • 컁오컁오
    98
    2021-01-05 00:31:28 작성 2021-01-05 00:33:20 수정됨

    저보다 나이가 있으셔서 지나가는 말로 들으셔도되는데, 지난 글들을 보니 굉장히 힘들게 살아오신 것 같습니다.

    저도 코드리뷰라는 것을 한 적은 없지만 코드리뷰라는게 발가벗겨지는 기분이란건 하나같이들 말하더라구요. 근데 '비난조'라는건 사실 이해를 못하겠습니다.

    들어가신 회사가 SI, SM이 아니라 서비스 쪽에 가까우신 거 같은데 보통 그런 회사들이 코드리뷰를 많이하는데, 서비스회사는 SI만큼 급하게 쫓기거나 하는 것이 없기 때문에 개발, 리팩토링을 즐기는 문화가 정착되어있는 편이라고 생각들거든요.

    그래서 굉장히 고인물들이 많고 한 사람이 일당백하는 케이스가 많아서 후배양성이 제대로 되지 않는 케이스도 있는 것 같구요. 그래서 그런 문화가 자리 잡힌 곳일 수록 교육을 시켜줄 때도 후배양성에 힘을 쓴다라기보다는 '어차피 내가 다 하면 그만인데 한두마디나 해주지 뭐' 하는게 어느 정도 있는 거 같거든요. 그런데 '비난조'로 말투가 나갔다는 거 자체가 사람이 문제거나 회사가 더 이상 성장의 여지가 없는건 아닐까 생각이 듭니다. 제가볼땐 회사가 할게없으면 다른 사람 갈구는게 사람 심리인거 같거든요. 보통 회사가 정착화되어있으면 내부에도 날파리가 꼬인다고 믿고있습니다.

    과거에 힘드신 일 때문에 직장 내 불협화음에 대해 수용하는 능력이 많이 생기신 거 같아서 아쉽네요. 속만 문드러지실터인데 ㅠ... 처음 코드리뷰때도 비난조였나요..? 그게 아니라면 노력만이 답인거 같긴 합니다.

  • 난나나난
    131
    2021-01-05 00:35:04

    어차피 프론트단에서도 한번 순회를 해야되서, 굳이 서버단에서 돌지 말고 앞단에서 하라는 거 같은데..

    프론트와 백엔드 롤에 대한 문제 같네요. 프론트단에서도 한 번 돌릴 정도로 작은 배열이라면 뒷단에서 한번 더 돈다고 뭔 문제가 있나 싶은 생각도 들고..  그냥 리뷰한답시고 쓸데없는 지적질 하는 것 같은데..

  • sulum
    119
    2021-01-05 00:47:44 작성 2021-01-05 00:57:48 수정됨
    조언해주셔서 감사합니다

    첫 리뷰때 아직도 기억이 나는데  3~4명에 둘러쌓여서 공개망신 주는 자리 같았던게 생생합니다. 멍청이가 된 기분이었습니다. 리뷰 시간도 2시간이 넘었고요.
    특히 팀장이 굉장히 한심스럽게 취급하더군요 ㅠ
  • 콰스웩스익조트
    468
    2021-01-05 00:53:18

    저런건 뭐... 어쩔수 없습니다.. 제가 봤을땐 리뷰어는 아마 다음과 같은 상태중 하나일거 같네요

    • 원래 리뷰하는 스타일이 저렇다. 
    • 리뷰어가 코드 리뷰 4 ~ 5 회 하는 동안 파악한 글쓴이의 능력이 기대에 미치지 못하였다 (이런거까지 설명해야 하나 이런 느낌?)
    • 초반에 일부러 강하게 리뷰하면서 글쓴이를 바른 길로 인도하려고 한다.
    • 과도한 업무로 심신이 지친 상태에서 코드 리뷰를 했는데 마음에 안든다.


    리뷰어가 첫번째 케이스만 아니라면... 글쓴이 하기 나름이라고 생각합니다. 

    리뷰어가 상대방 기분 안나쁘게 피드백 주는게 가장 좋겠지만... 어쩌겠습니까? 이미 닥친 현상인데.. 

  • sulum
    119
    2021-01-05 00:56:54 작성 2021-01-05 00:57:26 수정됨

    콰스웩스익조트


    조언해주셔서 감사합니다 ㅠㅠ 

    프로그래밍은 재밌는데

    멘탈이 약해서 살기가 쉽지않네요

  • 콰스웩스익조트
    468
    2021-01-05 01:01:04

    참고로 저도 리뷰 할땐 코드보고 할말 다 합니다.

    대신 거기에 사람을 엮으려고 하진 않습니다.

    최대한 작업한 코드만 보고 객관적인 피드백을 주려고 하다보니까 저도 말이 가끔 막 나가는 경우가 있습니다..

    물론 인지는 하고 있고 리뷰하기전에 막 말할수 있는거 양해 부탁드린다 라고 말은 하긴 하지만... ㅋㅋㅋㅋㅋㅋ 

  • deadde
    24
    2021-01-05 02:12:17
    그냥 인성의 문제인듯 하네요 ㅋㅋ 저같으면 좋은방법에 대한 질문을 해볼것 같습니다.
  • load2000
    3k
    2021-01-05 03:41:08

    제가 봤을때 거기 개발조직이 코드리뷰를 잘못 이해하고 있는거 같네요

    코드리뷰 커뮤니케이션 매너

    상당히 무례한 팀원들이군요


    코드리뷰의 자리는 절대로 누가 누구를 판단하고 검사 받는 자리가 아닙니다.

    공동의 코드에 대해서 같이 이해를 하는 자리입니다.

  • monotonicity
    97
    2021-01-05 08:08:00 작성 2021-01-05 08:10:03 수정됨

    작성자님 말대로 데이터 양이 2~10개 내외라면 루프 두번 도는걸로 효율성 지적하는건 많이 이상하다고 생각하네요. 요즘 컴퓨터 빠릅니다. 일반 PC도 몇십억개의 인스트럭션을 실행하는데 수초내에 끝낼수 있습니다. 일반적으로 서버 컴퓨터 성능은 이것보다 훨씬 빠르겠죠.

    소프트웨어 개발에 가장 중요한게 가독성과 유지보수성이라고 생각합니다. 효율성은 그 다음이구요. 위와 같은 micro optimization 을 한다고 해도, 0.0000001초 더 빨라지기나 할까요?

    효율성이 아닌 가독성 + 유지보수성의 이유로 저런 지적을 했다면 다른 얘기겠지만요.

  • jjna0825
    52
    2021-01-05 08:47:34

    계속 읽어보니 리뷰하는 사람이 그런 성향인것 같습니다.

    백앤드에서 처리하지 않고 프론트에서 처리했으면, 그런 정보처리는 백앤드에서 처리후에 푸론투에서는 기입만 해 주어야지 왜 프론트 처리를 했냐고 했을것 같습니다.

    저런 코드에서는 맞다/틀리다 라고 무조건 적으로 답을 내릴만한 요소는 아닌것 같으니까요.

  • 마조리카
    271
    2021-01-05 09:47:01

    근거를가지고 코드를 작성하면 좋을꺼 같다는 생각이 들어요.

    왜 화면단에서 안돌리고 서버에서 돌리느냐 (이렇게 작성한 이유를 말해라) 이거 아닐까요

  • onimusha
    8k
    2021-01-05 09:58:20
    표준 가이드가 선행되어야 코드리뷰를 받죠;;
  • 마스터key
    96
    2021-01-05 10:17:43

    사실 코드리뷰하다보면 발가벗겨진 기분 들 때도 있고, 동시에 왜 이따우로 짰냐고 입으로,

    혹은 눈으로 욕하는거 같고...  코드리뷰 처음하면 그렇게 다가올 때가 많습니다.

    그러다보니 힐난조로 들린다면 둘중에 하납니다.

    그 윗사람이 말하는 타입이 원체 그러거나, 아니면 진짜 잘못짠 소스코드라서 힐난하는거거나.

    그런데 다른사람도 그렇다면 그 회사 문화가 그런거구요.

    사실 둘다 커뮤니케이션 매너가 없는거긴 합니다만, 그건 사람이 바뀌지 않는 이상 힘들죠.


    그리고 정산얘기가 나오신걸로 봐선 어느정도 금액처리 쪽 데이터하고 연동되는 부분 페이지신거 같은데

    아무래도 돈이라는게 엮이게 되면 사람이 좀 민감해질 수 있습니다.

    코드 한줄 잘못썼다가 회사가 뒤집어질수도 있는 사안이니까요.


    코드리뷰에 적응되시면 아주 철판깔고 당당해지게 되구요.

    욕해도 아주 스무스하게 받아치게 되실겁니다 ㅋㅋ


  • 3인칭관찰자시점
    287
    2021-01-05 10:28:13
    틈틈이 팀원들의 코드를 체크해 보는것도 필요해 보입니다.

    작상자님 넘 자책하지 마세요
    언제가 그럼 니들이 짜던가 라고 맞대응 할 수 있는날이 올거에요.
  • 인사동
    1k
    2021-01-05 10:40:17

    코드리뷰가 원래 이런거냐고 물으신다면 ... 천차만별입니다.

    리뷰어도 능숙한 사람과 그렇지 않은 사람이 있습니다.

    현재 글쓴이는 해당 리뷰어의 방식이 이해가 잘안되는듯 합니다.

    물론 대부분 공감합니다. 글에 나온 리뷰어의 방식은 대체로 좀 이상합니다. 제일 중요한 매너(존중)도 찾아보기 힘들구요.

    하지만 뭐가 이상한지 내가 어떻게 대응해야 하는지 해당 리뷰어가 어떻게 바뀌었으면 좋을지에 대한 명확한 방법을 찾기 어려우실거라고 생각합니다.


    https://google.github.io/eng-practices/review/

    구글이 반드시 정답은 아닙니다만 대부분의 경우 좋은 문서를 가지고 있습니다.

    코드 리뷰를 요청하는측 (Code Reviewee) 와  리뷰하는 측 (Code Reviewer) 의 입장을 이해하시면 앞으로의 건설적인 대화가 수월해지지 않을까 생각합니다.


    ...라는게 제가 드리고 싶은 글이긴 합니다만 소화가 쉽지 않을수 있습니다 ㅎㅎㅎ; 저도 다 읽은건 아니구요...

    코드리뷰시 마음이 아프신거 같은데 전 code review 하거나 장애가 발생할 경우에도 해당 코드는 저만의 것이 아니라고 항상 생각합니다.

    코드의 문제점을 지적할때는 내 문제라고 생각해서 나를 자책하지 마시고 말고 내 코드의 문제(팀의 문제)이기에 같이 제3자의 관점에서 문제점을 찾고 지적하고 고칠점을 찾으면 맘아 아픈게 조금이나마 덜어지지 않을까 생각합니다.


  • kingofkj
    1k
    2021-01-05 10:46:39

    하도 오래되어서 php의 맵이 있는지 기억이 안나는데

    서버단에서 코드를 코드명으로 바꾸는 루프를 제거하시고

    코드와 코드명이 담겨있는 리스트를 key//value 형태의 맵이나 2차원 배열로 화면단에 던진후

    화면에서 해당 메인 리스트를 뿌릴때 코드명 객채를 리스트에서 변환하려 함께 보여주는 방식을

    원한거 같은데

    아니면 쿼리 단에서 한번에 코드명을 변환하는 리스트를 가져오거나요

    분명 방식에 문제는 있는 것으로 보입니다

    하지만 인격모독식으로 하는것은 잘못된 것이지만요

    문제의 해결방법을 하나하나 배워 나가면서 그런 조언은 삭히는 방법 밖에는 없는 것으로 보입니다

    지금은 참지만 나중에 잡아먹겠다는 독기는 가지면서요

  • 애아빠
    1k
    2021-01-05 11:08:57

    코드 리뷰를 하는게 아니라 사람을 리뷰하는 팀장님이시네요.

    누구나 비난 받으면 기분이 나쁩니다.

    결국 그게 더 생산성 저하로 이어지고 자존감도 떨어지지요.


    코드리뷰는 코드에 대해서 리뷰하는 겁니다.


    "너는 왜 이런것도 아직까지 못하냐. 너는 왜 이걸 이렇게 비효율적으로 짜놨냐."


    이렇게 하면 안됩니다.ㅠㅠ


    좋은 코드 리뷰란... 코드를 리뷰하는 것이라고 생각합니다.

    "이 코드는 이러 저러한 조건에 따라 동작하게 되는데 현재 코드는 이런저런 이슈가 있습니다. 이 코드가 이러한 형태로 이러쿵 저러쿵 변경이 된다면 이러저러한 면에서 더 나아질 수 있을 거 같습니다."


    라고 코드가 어떻다 라고 해야지. 그걸 만든 사람을 가지고 뭐라고 하면... 그냥 싸우자는 거지요.

  • 프록씨
    912
    2021-01-05 13:25:04 작성 2021-01-05 13:26:04 수정됨

    그 분이 코드리뷰를 본인 코드스타일대로 짜도록 강요하는 자리로 오해하고 계신거 같네요. 


    코드리뷰는 서로 의견을 공유하면서 발전하자는 취지이지, 본인 코딩스타일을 강요하는 자리가 아닙니다.


    그리고 개인적인 의견으로는 오히려 그런 내용이라면 서버쪽에서 모든걸 가공하고 프론트로 넘겨주는게 맞다고 생각하는 입장입니다.


    데이터가 한두건이면 상관없지만 데이터가 수천 수만건이면 프론트에서 그 데이터를 루프돌면서 변환하는건 사용자 입장에서 화면이 느리게 뜨는걸로 보일 여지가 있습니다.


    서버쪽이 연산도 빠를거고, 성능도 좋을 것이니 서버 쪽에서 연산하고 프론트에선 해당 데이터를 루프돌면서 뿌려주기만 하는 방식이 나을거 같습니다.





  • cleanning
    610
    2021-01-05 13:50:54 작성 2021-01-05 13:51:39 수정됨

    서버쪽에서 돌려서 주는게 맞지 않나요? 프론트에서 돌리면 렌더링 문제도 생길수 있는데 그분은 뭔 개소린지 모르겠어요. 연산되기 좋게 가상환경으로 구축된 서버에서 돌리는게 맞지않나?

    왜 앞에서 돌리래요?

  • 냥길동
    1k
    2021-01-05 16:27:49

    제가 들어왔던 코드리뷰하는 방식이 아닙니다. 코드리뷰할때 해서는 안되는 행동을 상대방이 하고 있습니다.

    앞으로도 바뀔여지는 없어보이므로 퇴사 추천드립니다. 자존감 엄청 떨어지게됩니다. 

  • sulum
    119
    2021-01-05 22:37:47 작성 2021-01-05 22:38:34 수정됨

    냥길동 자존감은 떨어지고 있긴합니다. ㅠ


    cleanning 프록씨 저는 단지 데이터 개수가 작기때문에 서버단에 구현했는데 말씀 들어보니 맞는말 같습니다.  

    애아빠 그런면이 있긴있는거 같습니다. 코드리뷰를 더 세게한다는 말도 들어봤습니다. 


     인사동 좋은글 링크해주셔서 감사합니다!..


    kingofkj 3인칭관찰자시점 monotonicity load2000  모두  말씀 감사합니다 !!..

     

    deadde  그런걸까요 ㅠ.ㅠ


     마스터key 그런날이 오면 좋겠네요.








  • Dive_Drink_Develope
    5k
    2021-01-06 09:30:36

    kingofkj 님 말씀대로 

    코드 : 이름 배열은 어차피 가져와야하는거고 

    어차피 화면단에서 포문 한번 돌려줘야하기때문에 그 배열에 그대로 키로 넣어서 던지면 별도 작업 없이

    화면에 뿌려주는데 백단에서 굳이 루프를 돌필요가 없어서 그런것같네요.

    뭐 스타일 차이라고 할 수도 있지만 그분이 짠 다른 코드들이 일반적으로 그런 형태를 띈다면

    그 통일성이 깨지는거 자체가 문제가 될 수 있죠.


    코드리뷰에 참석한 사람들한테까지 부끄러우실 정도면 회사 문화/그분과 안맞으시는거고

    본인 발전의 기회로 삼아 열심히 하시되 너무 인격적인 비난이 심하다고 생각되시면

    그분과 조용한 자리에서 따로

    지적해주신 것 감사하지만 공개적으로 ~~하는게 너무 부끄럽다. 

    코드에 대해서 지적하시는건 받아들이겠지만 조금만 부드럽게 말씀해달라고

    얘기 나눠보시는게 좋을것 같습니다.



  • 로그인을 하시면 댓글을 등록할 수 있습니다.