DonutWorks / Ari

0 stars 0 forks source link

#350 Feature/memo area to to notice #353

Closed MoojinChae closed 9 years ago

MoojinChae commented 9 years ago

350 Solved.

minhyeok4dev commented 9 years ago

인터페이스는 괜찮은것같은데 참가자 두명 이상일때 두번째 사람부터는 무조건 첫번째 사람의 버튼 색깔이 toggle되네.

예를들면 두번째 참가자의 출석, 회비 를 누르면 첫번째 사람의 버튼이 바뀌어. DB에는 정상적으로 잘 들어가고있는것같음.

원인은 그냥 $('#id') 이런식으로만 엘리먼트를 불러와서 그런것같은데, class로 불러올때와는 달리 id로 엘리먼트를 불러오면 수많은 동일 id 중에 첫번째것만 선택되더라. 그래서 그런듯!

MoojinChae commented 9 years ago

헛 민혁이가 말한거 수정했다! ㅎㅎ 감사감사

shaynekang commented 9 years ago

확인했습니다~

  1. jQuery 놔두고 onclick 같은거 쓰지 마세요. ㅋㅋ 왜 onclick을 쓰면 안되고 jQuery를 써야 하는지는 Unobtrusive JavaScript를 참고해주세요. ㅎㅎ
  2. CSS의 네이밍 컨벤션은 언더바(_)가 아닌 하이픈(-)입니다. 레일즈 뷰 헬퍼처럼 자동으로 생성해주는 경우만 언더바로 네이밍하고, 직접 작성하는 id, class명은 하이픈으로 해주는게 관례입니다. ㅎㅎ
  3. 네이밍을 좀 더 신경쓰면 좋을 것 같네요. <p id="memo_div">는 왜 div인지 애매한 표현 같습니다.
  4. 전반적으로 뷰에 로직 코드(모델 find/where 호출이라던가, if-else라던가)가 많은 것 같습니다. 로직 코드는 가능한 컨트롤러로 옮겨주시고, 복잡한 뷰는 데코레이터를 활용해주세요. ㅎㅎ
  5. 특히나 코드 중복이 많은 것 같습니다. views/admin/notices/_table_to_response.html.erb에서는 50줄짜리 뷰에 user.responses.find_by_notice_id(@notice)가 10개나 있네요. 신경써서 작성해주시면 좋겠습니다. ㅎㅎ

MVC 모델에서 코드를 작성할 때 가장 안 좋은 위치는 순서대로 뷰 > 컨트롤러 > 모델입니다.

  1. 뷰에 있는 코드는 다른 뷰에서 활용할 수 없고
  2. 컨트롤러에 있는 코드는 컨트롤러와 관계가 있는 뷰에서 사용할 수 있지만, 다른 컨트롤러에서는 사용할 수 없고
  3. 모델에 있는 코드는 모든 뷰, 컨트롤러, 심지어는 다른 모델에서도 활용할 수 있죠.

이번 Pull Request 같은 경우가 대표적인 예라고 보여지네요. 모델은 전혀 안 건드리고 전부 뷰-컨트롤러에서 처리를 하면 차후에 코드 재활용이 어렵고, 재활용이 안 되는 상황에서 다른 뷰에서 비슷한 기능을 구현하려고 하니 무의식적으로 복사-붙여넣기를 하게 되는거죠.

자주 말씀드리지만 어떻게 하면 지금 잘 돌아가는가를 넘어서서 어떻게 하면 앞으로도 잘 돌아갈 것인가를 고민해주시면 좋겠습니다. ㅎㅎ

shaynekang commented 9 years ago

아 그리고 분명 버그가 하나 있는데, 재현이 잘 안 되네요. ㅡ,.ㅡ;; 어디에 rand같은거 끼얹은거 아니여? ㅋㅋ

shaynekang commented 9 years ago

아 맞다! 메모창에서 엔터를 누르면 안 된다고 했구나. ㅋㅋ 여튼 요 엔터 버그는 좀 고쳐주세요. ㅡ,.ㅡ

18_notice_memo_error

메모창에 focus가 있는 상태에서 엔터를 누르면 정상적으로 작동하면 좋겠습니다. ㅎㅎ