이전 게시글에서 복수 파일을 다루기 위해

ArrayList에 담고 복잡한 로직을 구현했었다

https://vuddus526.tistory.com/504

 

[Java] java.nio 활용 FileTracker 구현 2 (복수 파일 처리)

단일 파일 처리 하는 방법을 적은 게시글에서 기본적으로 사용되는 Path, WatchService, FileChannel 들의 내용을 적어두었다 해당 게시글에서는 기존에 만든 코드에서 복수개로 처리할때 신경쓰고 처리

vuddus526.tistory.com

 

그런데 코드리뷰를 하면서 살펴보니 그럴필요가 없다는걸 알게되었다

 

애초에 hashMap에 값을 담았는데 굳이 ArrayList에 또 담을 필요가 없는것이다

 

지금 구조를 보자면 아래와 같다

1 - key - value

2 - key - value

3 - key - value

 

그렇기 때문에 굳이 key로 값을 찾아낼 수 있는데

순서를 보장해야하는것 아닌이상 arrayList에 담을 필요가 없는것이다

 

지금 상황에서는 순서를 보장할 필요가 없으니...

 

그래서 리팩토링을 진행했고 해당 부분 이외에도

변수명 이라던지 중복되는 코드 처리를 했다

 

1. 리팩토링한 내용 정리

1) for문 내 변수명 정정

정확히 어떤 값인지 변수 명을 정확히 하는게 좋다는 피드백을 반영

 

변경 전 for문

for (int i = 0; i < this.target.size(); i++) {
	// 생략
}

 

변경 후 for문

for (int pathIndex = 0; pathIndex < this.target.size(); pathIndex++) {
	// 생략
}

 

2) HashMap 만 사용

기존에는 ArrayList<HashMap<Path, FileChannel>> 형태 였다면

HashMap<Path, FileChannel> 만 사용하였다

그렇게 했더니 for문도 써줘야하고 20줄 가까이 써야했던 코드가

for문 없이 5줄만 써도 되는 코드로 바뀌었다

 

변경 전

if (kind == StandardWatchEventKinds.ENTRY_CREATE) {
	for (int i = 0; i < this.readChannel.size(); i++) {
    	FileChannel tempReadChannel = this.readChannel.get(i).get(imsiEventFile);
        
		// 기존 채널 닫음
		if (tempReadChannel != null) {
			tempReadChannel.close();
			tempReadChannel = null;

			// 기존 채널 삭제
			this.readChannel.remove(i);
			this.onlyReadChannel.remove(i);
			this.target.remove(i);

			// 새로운 채널 추가
			this.target.add(i, parentPath.resolve(eventFile));
			this.pathFileChannel = new HashMap<>();
			this.pathFileChannel.put(eventFile,
				FileChannel.open(this.target.get(i), StandardOpenOption.READ));
			this.readChannel.add(i, this.pathFileChannel);
			this.onlyReadChannel.add(i, this.readChannel.get(i).get(eventFile));
            
    }
}

 

변경 후

if (kind == StandardWatchEventKinds.ENTRY_CREATE) {
					
    // 파일경로 = 파일 루트 경로 + 새로 생성된 파일 이름
    Path imsiTarget = parentPath.resolve(eventFile);

    // 새로운 파일 경로 target 에 추가
    this.target.add(imsiTarget);
    // 새로운 채널 열면서 pathFileChannel에 추가
    this.pathFileChannel.put(eventFile, FileChannel.open(imsiTarget, StandardOpenOption.READ));
    // 새로운 파일 끝에서 읽게 처리
    long offset = imsiTarget.toFile().length();
    this.pathFileChannel.get(eventFile).position(offset);

}

 

3) 파일의 끝에서부터 시작

DB에 로그 데이터를 삽입하는 과정에서

로그파일에 기존에 남아있던 데이터가 있다면

새로운 데이터가 들어오는 시점에 기존 데이터가

일괄 DB에 삽입되는 이슈가 발생했다

 

확인결과 채널이 생성될때 파일의 끝에서부터 읽게 처리하는 로직이

잘못되었고 정확히는 파일의 전체 경로를 지정해줘야 하는데

파일의 이름만을 지정해주고 있었다

 

변경 전

long offset = target.toFile().length();
fileName.position(offset);	// 파일의 이름만

 

변경 후

long offset = target.toFile().length();
this.pathFileChannel.get(fileName).position(offset);	// 파일의 전체경로

 

 

2. 느낀점

처음부터 알아 차리고 코드를 짰다면 시간이 훨씬 줄어들었을텐데

그 당시에는 왜 HashMap을 ArrayList에 담아야한다고 생각한지 모르겠다..

이번 경험을 통해 구현 전 더 신중하게 생각해야겠다고 느꼈고

경험이 많은 팀장님의 코드리뷰 덕분에 모르거나 까먹었던 내용을

다시 상기하며 클린코드에 대해 생각하는 기회가 된거 같다

 

옛날에 짜놨던 코드도 다시 보며 리팩토링하는 시간을 가져봐야겠다