PMD. AvoidInstantiatingObjectsInLoops에 대해서 어떻게 생각하시는지요?
임성현
설마 이런 것도 실수할까 싶은 내용도 있습니다.(자세한 사항은 추후에.)
그 중에 다양한 환경과 상황에 따라서 의견이 분분한 룰이 AvoidInstantiatingObjectsInLoops 입니다.
자세한 설명을 살펴보면,..
** AvoidInstantiatingObjectsInLoops
------------------------------------------------------------------------------------------------------------
Since: PMD 2.2
Detects when a new object is created inside a loop
This rule is defined by the following Java
class:net.sourceforge.pmd.rules.optimization.AvoidInstantiatingObjectsInLoops
Example:
public class Something {
public static void main( String as[] ) {
for (int i = 0; i < 10; i++) {
Foo f = new Foo(); //Avoid this whenever you can it's really
expensive
}
}
}
------------------------------------------------------------------------------------------------------------
이라고 나오고, Optimization Rule에 해당하는 사항입니다.
간단하게 말씀드려서 loop안에서 반복적으로 사용하는 객체의 경우 루프 밖에서 생성하고 재활용하라는 의미인데.
아래 메소드를 한번 보시죠.(Service에서 사용하는 메소드 입니다.)
public List sampleAbcList(List inList){//DB에서 DVO에 정보를 넣어서 List로 전달
List returnList = new ArrayList();
Iterator itr = inList.iterator();
while(itr.hasNext()){
//리스트에서 하나씩 꺼내서 가공함
AbcDVO dvo = new AbcDVO(); //--------(1)
dvo = (AbcDVO) itr.next();
dvo.setLoginEpno(session.getLognEpno());
dvo.setScnId(session.getScnId());
dvo.setLastChgPsEpno(session.getEpno());
returnList.add(dvo);
}
return returnList;
}
위의 경우, inList는 DB에서 정보를 가져온 내용이고, 이를 하나씩 꺼내서 가공해서 다시 넣는 경우인데..
이 경우 처음 말씀드린 Rule을 활용해서 객체생성(1)을 while 밖에서 생성해서 재활용해야 한다 라고 가이드하는 것이
1) 문제점이 없는지
2) 이득을 얻을 수 있는지
라는 측면이 이슈 입니다.
rule 위반을 해결하려면 아래와 같이 작성하는데,
public List sampleAbcList(List inList){//DB에서 DVO에 정보를 넣어서 List로 전달
List returnList = new ArrayList();
Iterator itr = inList.iterator();
AbcDVO dvo = new AbcDVO(); //--------(1)
while(itr.hasNext()){
//리스트에서 하나씩 꺼내서 가공함
dvo = (AbcDVO) itr.next();
dvo.setLoginEpno(session.getLognEpno());
dvo.setScnId(session.getScnId());
dvo.setLastChgPsEpno(session.getEpno());
returnList.add(dvo);
}
return returnList;
}
이 부분에 대해서 제 의견은 이렇습니다.
(1) 문제점은 없는지...
- abcDVO는 이미 DAO에서 만들어서 inList로 담아서 보내고 있고
- 이를 (1)에서 생성된 객체는 단지 받아서 전달만 하므로 returnList에 담기는 객체는 inList의 DVO임
- 따라서, (1)의 dvo는 원본 객체의 레퍼런스만 담고 있는 객체가 됨
- 재활용에 문제 없음
(2) 이득을 얻을 수 있는지
- 개발자의 습관과 경험을 바꿀 정도로 의미가 있을까...의 사항인데,
위 내용은 보통 조회성 화면의 게시판 테이블에 담기는 경우가 대부분입니다.
- 즉, List의 사이즈가 많아도 20~40개 정도 만들어지는데.... 이를 큰 오류라고 생각하고 해결하기 위한
노력을 하는 것이 중요한가. 라는 부분 입니다.
- 여기에서는 다소간의 견해 차이가 존재할 수 있습니다만, 제 생각에는
확실한 사항에 대해서만 개발자의 습관을 바꾸도록 한다
(예: obj.equals("String") --> "String".equals(obj) 으로 null point
exception 방지 등)
입니다.
따라서, 내부적으로는 위 사항을 감안하여 결과를 확인하고, 조치는 강제사항이 아닌
권고사항으로 접근하는것이 좋겠다는 생각입니다.
다른 분 의견은 어떠하신지요?....
심상호
List returnList = new ArrayList();
Iterator itr = inList.iterator();
Sanghyuk Jung
List returnList = new ArrayList();
Iterator itr = inList.iterator();
while(itr.hasNext()){
//리스트에서 하나씩 꺼내서 가공함
AbcDVO dvo = new AbcDVO(); //--------(1)
dvo = (AbcDVO) itr.next();
dvo.setLoginEpno(session.getLognEpno());
dvo.setScnId(session.getScnId());
dvo.setLastChgPsEpno(session.getEpno());
returnList.add(dvo);
}
return returnList;
}
List returnList = new ArrayList();
Iterator itr = inList.iterator();
AbcDVO dvo = new AbcDVO(); //--------(1)
while(itr.hasNext()){
//리스트에서 하나씩 꺼내서 가공함
dvo = (AbcDVO) itr.next();
dvo.setLoginEpno(session.getLognEpno());
dvo.setScnId(session.getScnId());
dvo.setLastChgPsEpno(session.getEpno());
returnList.add(dvo);
}
return returnList;
}
AbcDVO dvo = null;
--
Google 그룹스 'Korea Spring User Group' 그룹에 가입했으므로 본 메일이 전송되었습니다.
이 그룹에 게시하려면 ks...@googlegroups.com(으)로 이메일을 보내세요.
그룹에서 탈퇴하려면 ksug+uns...@googlegroups.com로 이메일을 보내주세요.
더 많은 옵션을 보려면 http://groups.google.com/group/ksug?hl=ko에서 그룹을 방문하세요.
Sungchul Park
매개변수로 넘어 온 리스트에서 꺼낸 객체를 가공하고 새 개체에 넘기는 사이
에 원본이 변조되는데요.
sampleAbcList()를 호출한 쪽에서 내가 매개변수로 넘긴 리스트가 변조될 수
있다고 가정하는 일은 거의 없는 듯 합니다. 일종의 최소 놀람 원칙 위반이랄
까요? 불변객체를 사용해 파급 효과를 최소화 하는 관례가 요즘 강조되고 있
습니다.
Sanghyuk Jung
while(itr.hasNext()){
//리스트에서 하나씩 꺼내서 가공함
dvo.setScnId(session.getScnId());
dvo.setLastChgPsEpno(session.getEpno());
returnList.add(dvo);
}
return returnList;
Sungchul Park
원래 제시하려고 했던 소스는 이런 게 아닐가요?
public List sampleAbcList(List inList){//DB에서 DVO에 정보를 넣어서 List로 전달
List returnList = new ArrayList();
Iterator itr = inList.iterator();
while(itr.hasNext()){
//리스트에서 하나씩 꺼내서 가공함
AbcDVO dvo = new AbcDVO((AbcDVO) itr.next());
dvo.setLoginEpno(session.getLognEpno());
dvo.setScnId(session.getScnId());
dvo.setLastChgPsEpno(session.getEpno());
returnList.add(dvo);
}
return returnList;
}
inList에서 하나씩 받아서 이것을 복사한 다음에 (이 과정에 새 객체가 생성
되죠) 이를 반환할 리스트(returnList)에 넣고 반환하는...
> 위의 경우, inList는 DB에서 정보를 가져온 내용이고, 이를 하나씩 꺼내서 가공해서 다시 넣는 경우인데..
> 이 경우 처음 말씀드린 Rule을 활용해서 객체생성(1)을 while 밖에서 생성해서 재활용해야 한다 라고 가이드하는 것이
> 1) 문제점이 없는지
이 경우는 PMD의 룰이 잘못입니다. PMD가 더 똑똑했다면 지적하지 않았을 것
같네요.
> 2) 이득을 얻을 수 있는지
객체 생성을 밖으로 뺄 수 없습니다.
> rule 위반을 해결하려면 아래와 같이 작성하는데,
>
> public List sampleAbcList(List inList){//DB에서 DVO에 정보를 넣어서 List로 전달
> List returnList = new ArrayList();
> Iterator itr = inList.iterator();
> AbcDVO dvo = new AbcDVO(); //--------(1)
> while(itr.hasNext()){
> //리스트에서 하나씩 꺼내서 가공함
> dvo = (AbcDVO) itr.next();
> dvo.setLoginEpno(session.getLognEpno());
> dvo.setScnId(session.getScnId());
> dvo.setLastChgPsEpno(session.getEpno());
> returnList.add(dvo);
> }
> return returnList;
> }
>
> 이 부분에 대해서 제 의견은 이렇습니다.
> (1) 문제점은 없는지...
루프 밖에서 생성한 객체는 전혀 사용되지 않습니다.
> (2) 이득을 얻을 수 있는지
1번 소스의 원본에서 new한 객체도 이번에 밖으로 뺀 객체도 전혀 사용되지
않습니다. 오류이니 이득을 따질 필요가 없네요.
> - 즉, List의 사이즈가 많아도 20~40개 정도 만들어지는데.... 이를 큰 오류라고 생각하고 해결하기 위한 노력을 하는 것이 중요한가. 라는 부분 입니다.
20-40 정도되는 리스트를 만드는 게 VM 입장에서 큰 부하는 아니라고 생각합
니다. 필요하면 만들어야겠죠. 하지만 애초에 원본 소스에 오류가 있어 성능
을 논할 단계는 아니라고 생각합니다.
JUNG-LAE JO
임성현
결국 보안에도 문제있고(참조하는 원본이 변경될 수 있고),
Loop에 들어가자마자 바로 다른 객체를 참조하는데 내부에서든 외부에서든 생성할
가치가 없다 라는 측면으로 이해 됩니다.
성철님의 말씀(내가 처리를 요청한 원본이 의도하지 않게 변해버린다)라는 측면은
좀 더 진지하게 고민해볼께요. 어떻게 사용되는지.. 좀 더 살펴봐야겠습니다.
원래의도인 룰 위반을 하는지 여부 보다는
어떤 이유로 이런 형식의 코딩을 하는지 지켜보는 용도로 써야겠군요.
한시간도 안되어서 이렇게 뜨거운. 호응을 받으니.
감사 합니다.
> > 그룹에서 탈퇴하려면 ksug+uns...@googlegroups.com<ksug%2Bunsu...@googlegroups.com>로
Sungchul Park
> 원본을 변조하지 않으려면 new나 clone 같은 것이 당연히 필요하겠죠.
> 그렇게 하려면 아래와 같이 코드가 바뀌어야할 것 같습니다.
> while(itr.hasNext()){
> //리스트에서 하나씩 꺼내서 가공함
> AbcDVO source = (AbcDVO) itr.next();
> AbcDVO dvo = new AbcDVO (source.getId().....
> 등등 초기화 값);
> dvo.setLoginEpno(session.getLognEpno());
> dvo.setScnId(session.getScnId());
> dvo.setLastChgPsEpno(session.getEpno());
> returnList.add(dvo);
> }
> return returnList;
>
> 그리고 이 경우에도 new 생성말고 dvo 선언 자체를 밖으로 빼는 것은 아무
> 성능에 이득이 없습니다. 변수범위를 최소화하는 것이 좋겠죠.
애초에 PMD의 경고가 반복문 안에서 객체를 생성하는 게 옳은지 여부인데 사
실 올리신 소스에서는 애초에 객체 생성이 무의미 하기 때문에 객체 생성이
반복문 안에서 의미있게 사용되는 사례로 좀 바꾸어 본 것입니다. 논점이 소
스의 문제로 바뀐 것 같아서요.
임성현님이 토론하기 원하시는 것은 루프안에서 객체를 생성함으로써 PMD의
경고를 받았는데 PMD의 경고를 무시해도 좋은지, 아니면 PMD의 지침을 따라
변경해야 하는지 같습니다.
Sungchul Park
> 처음 질문을 보면.. 서비스 단인데 굳이 원본을 복사할 필요가 있을까요?
에서 그것을 미리 고려하고 있을테니까요. 시스템이 복잡해지면 이런 선택이
문제를 일으킬 수 있지만 웹은 단순하고 객체의 생애가 짧기 때문에 대부분
문제가 생길 일이 없겠죠.
하지만 단순한 DTO가 아닌 ORM의 영속화된 객체라면 고민해야 할 부분입니다.
그리고 제가 원본을 복사하는 버전으로 바꾼 이유는 임성현님의 질문 의도를
더 부각하려고 한 것 뿐입니다.