2008. 12. 16. 20:29

Helols님 게시판 코드 살펴 보기

제가 스터디 초반에 다른 사람들의 코드를 보고 리뷰를 하자고 했는데 사실상 스터디 시간에 리뷰를 하기란 참으로 빠듯하더군요. 그렇쵸? 그래서 지금 소스 관리 저장소에 올라온 성윤님 코드를 받아서 잠깐 들여다 봤습니다.(다른 분들 코드도 올려주시면 좋겠네요. 아란님과 써니님 게시판 소스도 보고파요.)


메이븐 프로젝트보다 일반 프로제긑가 좋은 점
- 대부분의 경우 JRE 설정만 잡아주면 프로젝트에 에러는 없다.

안 좋은 점
- SVN에서 lib 파일 받느라 체크 아웃이 조금 오래 걸린다.
아직 메이븐을...접해보지 못했어요;;ㅋㅋ


설정파일이 뭔가.. 이상함...
- applicaionContext.xml이라고 되어 있어야 할 것들이 모듈별로 여러 개의 xxx-servlet 으로 쪼개져 있고 xxx-servlet 에 들어가있어야 할 것이 applicationCotnext.xml 이라는 이름의 설정 파일에 들어가 있음. @_@
- 성윤님께서 스프링 MVC에서 applicatonContext와 dispatcherServlet이 사용하는 webapplicationContext 계층 구조를 햇갈리신 걸까? 아니면 어떤 의도로 저렇게 나눠두신 걸까??
- 설정 파일의 위치도 이상함. applicationContext.xml은 보통 src 폴더에 두어야될 것 같고(설정 파일은 관련된 코드 근처에 두는게 좋겠다는 취지에서..) xxx-servlet은 WEB-INF 밑에 두면 될 것 같은데.. applicationContext가 WEB-INF 밑에 있네요. test 폴더 밑에는 테스트용 applicationContext가 있어야 하는데 그 이름이 역시 test-servlet이라고 되어 있어서.. 뭔가 좀.. 이름이 별로네요.
푸핫;; 어떻게 하다보니.. ㅋ 우선.. 머라고 할까;; xxx-servlet.xml 파일들이 WEB-INF밑에 바로가 있는 모습이 맘에 들지 않아서 폴더를 하나 만들어서 그곳에서 또 모듈 별로 설정 파일을 나누려는 목적이었으나..
게시판에는 몇개의 설정파일들이 존재 하지 않게 되어서..ㅋ 구지 따로 옮길 필요 까지는 없었는데;;(따로 옮기면서 web.xml에도 구지 해당 xml위치를 적어주어야 했음.) ;; 전에 프로젝트 할때 .. 너무 많은 설정 파일들이 산재하게 되어;; 어떻게 정리가 되지 않을까 싶어서 시도해보았고..xxx-servlet.xml 파일내용과 applicationContext.xml 파일 설정내용이 좀 이상 애매모호 하게 된건.. xxx-servlet.xml 설정파일을 나누다 보니 공통적으로 적용될건 applicationContext.xml로 빼버리게 되엇는데;; 나중에는 init-servlet.xml 파일이 하나 생기고 ㅡㅡ;; 뭐 이래저래 막무가내로 코딩하게 되었네요;;ㅋ test관련해선.. 걍 즉흥적으로;;ㅋㅋ


인터페이스 사용 일관성 문제
- DAO는 인터페이스를 사용했으나 서비스에는 사용하지 않았네요. 사용하려면 다 쓰고 안 쓰려면 다 안 쓰지. 왜 그렇게 하셨을까요?(참고로 전 다 안 썼답니다.ㅋㅋㅋㅋ)
ㅋㅋ 원래 기존에는 서비스와 DAO에 전부 인터페이스가 자리 하고 있었으나... 문득 드는 생각이 이놈이 여기 왜 있을까;;; 해서 없애 버렸음..ㅋ 그나마 DAO는 기존 JDBC로 구현한놈과 아이베티스를 적용해서 구현한놈을 둘다 가져가볼까 해서;;ㅋㅋ 두었는데;; 뭐 아직 미완성이라;;ㅋ

컨트롤러에서 유일한 Servlet API = HttpSession.
- 2.X 대에서는 불가피하지만, 3.0부터는 @Session인가 하는 애노테이션을 사용해서 세션에 있는 attibute를 자동으로 메서드 매개변수로 바인딩해서 사용할 수 있다고 하네요.
굳 인데요;;ㅋ

request mapping 범위가 너무 넓은거 아닐까요?
-     @RequestMapping("/*/category/*")
    public String getCategoryInPostSch(HttpServletRequest req,HttpSession session, ModelMap map)
    { ...

이런 코드가 있던데 저 메소드가 실제로는 요청 하나만 처리할 텐데 범위가 *로 너무 넓게 되어 있는 듯 합니다.

이놈은 /*/category/* 이건 ... 화면에서 넘길때 .. /home/해당유저id/catagory/(카테고리 번호) 를 받는 놈이라서 * 로 했어요~ 그러니가;; 보통 catagoryID를 파리미터로 넘기지 않고 저런식으로 넘겨보려고요;;ㅋ
티스토리가 저런식이길래;; 흉내를 내보았네욥;ㅋ 그런데 저런 것 때문에 한동안 마니 삽질을 했다는..ㅋ
 /*/category/여기 뒤에는 단순히 카테고리 번호만 옴!

굳이 세터는 만든 이유는?
-     @Autowired
    public void setBbsDao(BbsDao bbsDao)
    {
        this.bbsDao = bbsDao;
    }...

Category 클래스에 이런 코드가 있었습니다. 세터 없이 그냥 @Autowired를 필드에 붙여주면 되는데 왜 세터를 만들고 메소드 위에 붙여두셨죠? 세터가 필요한 이유라도 있는건가요?

카테로리 클래스에 private  BbsDao bbsDao; 요넘의 접근제어자를 private 로 두고 test코드를 작성하다 보니.. 불가피 하게 set 메소드가 생겼네욥;; mock 객체를 대신 넣으려구요...ㅋ

잘 이해가 안 되는 코드
-     public Category getCategoryName(Map<String, String> paramMap)
    {
        bbsId  = paramMap.get("bbsId");
        cKey   = paramMap.get("cKey");
        fullYn = paramMap.get("fullYn");
        bbsDao.getCategoryName(this);
        return this;
    }

이 코드는 무슨 일을 하는거죠? 보통 get으로 시작하는 메소드는 뭔가 리턴하고 그 리턴한 걸 받아서 쓰는 코드를 예상하기 쉬운데 위에 있는 getCate.. 를 따라가 봤더니 주석에 // 쓰레기 코드라고 되어 있네요. 크헉...ㅋㅋ

이놈은 음.. 카테로리 네임 정보를 가져 오는 놈인데요;;ㅋㅋ
저기 맵에서 꺼내는건;; 마땅히 컨트럴러에서 받아온값을 서비스계층으로 넘기고 그값들을.. 서비스 계층에서 담으려다가 걍;; 도메인 계층에서 한번 담아 본거였어요;;ㅋㅋ 물론 어정정하게 DDD를 해보겠다고 해서 들여댔다가 나온 파생코드죠;;ㅋㅋ 쑤레기 코드..ㅋ

코드 스타일
- 저는 보통

public void do() {
 /// 요렇게 이클립스 기본 스타일로 쓰는데..
}

성윤님은

public void do()
{
// 요렇게 비주얼베이직 기본 스타일처럼 쓰시네요.
}
ㅋㅋ 어떻게 하다보니... 습관이 저렇게 들어버렸네요;; if else if esle 뭐 이런것들 적을때;;
명확히 구분하기가 힘들어 보여서... 시작했던 코딩스타일인데요;; 시작하다보니.. 저렇게 굳어버렷네요;;
가끔은 헛 공간을 잡아먹어서;;ㅡㅡ;

세터에 null 방지 코드
-     public void setPostReplyYn(String postReplyYn)
    {
        this.postReplyYn =postReplyYn == null ? "N":postReplyYn;
    }

이런 코드가 있던데.. 아무래도 null 방지 용으로 세터에 손을 쓴 것 같습니다. 음~ null 방지 코드라.. validation인지.. 기본값 세팅인지..흠.. 뭔가 좀 생각을 해봐야겠네요. 일반적인 기본 세터랑 차이가 있어서 당연히 기본 세터 겠거니하고 코딩을 하다가는 예상치 못한 결과를 볼 수 도 있겠군요.

화면에서 널이 넘어 와서 널은 N으로 치환 해줘야 하는 걸 해주어야 하는데;; 쿼리에서 NVL로 할까;; 아님 다른데서 널치환을 할까 하다가 ;;; 여기에 자리 잡았네요;; ㅋ

왜 맵을 통쨰로 던졌을까?
-     public boolean checkFullCategory(Map<String, String> paramMap)
    {
        return bbsDao.checkFullCategory(paramMap).get("FULLYN").equals("Y")?true:false;
    }
이 코드를 보면 필요한 값만 넘기지 않고 맵을 통째로 던져서 코드가 길어진 걸 볼 수 있습니다. 물론 3항 연산자로 한 줄로 끝내긴 했지만. 그래도..왜 그런 원칙이 있자나요. 드리트리 원칙이던가.. 친한 친구랑만 얘기하라던.. 그걸 지킬 수도 있는데 위반한 대표적인 코드가 되겠네요.

음 이놈은... 해당 넘어온 카테고리 번호가 전체 카테고리 번호인지를 체크하는 놈인데요... 우선 맵을 던진이유는 해당 인자값에 해당하는 놈이 전체 카테고리에 해당하는 놈인지 쿼리를 한번 날리고 그 결과값이 맵으로 리턴되어서 그 맵안에 FULL_YN 값이 Y 이냐 N 이냐에 따라 달라지는 코드인데;; 
public boolean checkFullCategory(Map<String, String> paramMap)
    {
Map returnMap = bbsDao.checkFullCategory(paramMap);
if(returnMap.get("FULLYN").equals("Y")) return true;
return false;
    }

이렇게 되어 지네요;; 결과가 Map으로 리턴이 되어서 인자로 넘긴 맵의 값을 꺼내서 체크 하는것처럼 절못보였는건가;;; 음.. 일단 ;; 페스~ㅋㅋ

트랜잭션 코드가 안보이네요.
- @Transactional 이라는게 안보이네요. transactionManager 빈이 등록된 건 확인을 했는데 말이죠. 흠. 트랜잭션 처리는 어떻게 하고 계신가요?
일단 멍석만..ㅋㅋ 다음 코드 리뷰때 아마 등장할듯합니다..ㅋㅋ

HelolsResponse라는 클래스 이름이 맘에 안 듭니다.
- HeloIs 때문이 아니라 위에 서픽스가 영..;; 이 클래스가 AbstractView를 상속 받았다면 이 녀석도 View로 끝내는게 좋치 않을까요?

ㅋㅋ 영작하는데 소질이 없어서요;;ㅋ 어떤 이름을 지을까 이래 저래 하다가 ;;; 먼가 하나 남겨 볼까 해서;;ㅋ 넣어봤죠;;^^;;; 그 머냐 ... 낙서기질이라고 할까욥;;ㅋㅋ

이상.. 다소 까칠하게 쓰기도 했지만 좋게 봐주시리라 생각합니다. ^^

전체적으로 Map 사용과 3항 연산자 코드가 눈에 띄네요. 캬~ 역시 소스는 직접 다운받아서 봐야 잘 보이네요. 다른 분들도 받아서 함 봐 보시죠. 저장소가 저희 회사 서버에 있어서 굉장히 빠르답니다.ㅋㅋ