[Refactor] - TDD 서비스 코드를 간결하게 해보자,,!

 

이번에 제가 필요해서 시작한 프로젝트인 TDD 프로젝트의 코드 개선 과정에 대해 포스팅을 하겠습니다.

 

우선 TDD프로젝트는 개발자를 위한 TodoList서비스이고, 사용자가 TodoList에 할 일을 추가하면 깃허브 레포에 자동으로 이슈와 리드미가 생기게 되는 서비스입니다. (취준생은 잔디도 관리를 해줘야되기 때문에,,ㅎ)

 

그래서 이번 프로젝트에서는 대부분 Git API를 사용하는 코드가 많기 때문에 최대한 간결하게 정리를 해보고 싶었다.

우선 처음 시도했던 코드를 보겠습니다.

 

fun createTodoList(
        todoRequest: TodoRequest,
        username: String
    ): Long? {
        val member = memberReader.getMember(username)

        ...

        //이슈 템플릿 작성
        val gitHubIssuesRequest =
            gitHubIssueCreator.createIssueTemplate(todoRequest.toTodoCreate(member))

        // 이슈 생성
        val createIssue = gitHubIssueProcessor.createIssue(
            member.gitHubToken,
            member.username,
            member.gitHubRepo!!,
            gitHubIssuesRequest
        )

       ...
        //리드미 파일 작성
        readMeProcessor.generatorReadMe(
            member.gitHubToken.toGeneratorBearerToken(),
            member,
            member.gitHubRepo!!
        )
       ...
    }

    fun finishTodoList(todoListId: Long, username: String) {
      ...
        val issueUpdateRequest = GitHubIssueStateRequest(state = GitHubService.ISSUE_CLOSED)
        gitHubIssueProcessor.closeIssue(
            member.gitHubToken,
            username,
            member.gitHubRepo!!,
            memberTodoList.issueNumber!!,
            issueUpdateRequest
        )
        readMeProcessor.generatorReadMe(
            member.gitHubToken.toGeneratorBearerToken(),
            member,
            member.gitHubRepo!!
        )
    }

    fun modifyTodoList(todoListId: Long, todoRequest: TodoRequest, username: String) {
   ...

        val gitHubIssuesRequest =
            gitHubIssueCreator.createIssueTemplate(todoRequest.toTodoCreate(member))

        gitHubIssueProcessor.updateIssue(
            member.gitHubToken.toGeneratorBearerToken(),
            member.username,
            member.gitHubRepo!!,
            todoList.issueNumber!!,
            gitHubIssuesRequest
        )
        readMeProcessor.generatorReadMe(
            member.gitHubToken.toGeneratorBearerToken(),
            member,
            member.gitHubRepo!!
        )
    }

    fun removeTodoList(todoListId: Long, username: String) {
      ...
        val issueUpdateRequest = GitHubIssueStateRequest(state = GitHubService.ISSUE_CLOSED)
        gitHubIssueProcessor.closeIssue(
            member.gitHubToken,
            username,
            member.gitHubRepo!!,
            todoList.issueNumber!!,
            issueUpdateRequest
        )
        readMeProcessor.generatorReadMe(
            member.gitHubToken.toGeneratorBearerToken(),
            member,
            member.gitHubRepo!!
        )
    }
    
    fun unFinishedTodoList(todoListId: Long, username: String) {
        ...

        val issueUpdateRequest = GitHubIssueStateRequest(state = GitHubService.ISSUE_OPEN)
        gitHubIssueProcessor.closeIssue(
            member.gitHubToken,
            username,
            member.gitHubRepo!!,
            findTodoList.issueNumber!!,
            issueUpdateRequest
        )
        readMeProcessor.generatorReadMe(
            member.gitHubToken.toGeneratorBearerToken(),
            member,
            member.gitHubRepo!!
        )
    }

위의 코드를 보면 ~er를 통해서 깃 관련 작업을 하는 클래스를 분리하고 호출하는 형식을 사용하였지만 제 눈에는 메소드마다 공통적으로 선언을 하는 부분이 너무 지저분 했고, 좀 더 간결하게 할 방법이 없을까 라는 생각을 하였습니다. (중복 코드가 너무 많아,,,,!!!!)

 

저는 특정 작업이 발생했을 때 그 작업에 맞는 이벤트가 발생하게 하면 어떨까하는 생각에 EventListener를 활용해서 해보는건 어떨까 라는 생각을 하였습니다.

그렇게 되면 위 코드처럼 매 메소드마다 ~er가 사라지고 좀 더 간결해지지 않을까? 라는 생각을 하고 바로 코드에 작성을 해보았습니다...

이 엄청난 의존성을 최대한 줄이고 싶은 내마음,,

 

그렇게 전부 EventListener로 수정한 결과,,,!

fun createTodoList(
        todoRequest: TodoRequest,
        username: String
    ): Long? {
       ...
        eventPublisher.publishEvent(issueEventRequest)
       ...
        eventPublisher.publishEvent(ReadMeEventRequest(member))
        return todoListId
    }

    fun finishTodoList(todoListId: Long, username: String) {
        ...
        eventPublisher.publishEvent(issueCloseEventRequest)
        eventPublisher.publishEvent(ReadMeEventRequest(member))
    }

    fun modifyTodoList(todoListId: Long, todoRequest: TodoRequest, username: String) {
        ...
        eventPublisher.publishEvent(issueUpdateEventRequest)
        eventPublisher.publishEvent(ReadMeEventRequest(member))
    }

    fun removeTodoList(todoListId: Long, username: String) {
        ...
        eventPublisher.publishEvent(issueCloseEventRequest)
        eventPublisher.publishEvent(ReadMeEventRequest(member))
    }

    fun unFinishedTodoList(todoListId: Long, username: String) {
      ...
        eventPublisher.publishEvent(issueCloseEventRequest)
        eventPublisher.publishEvent(ReadMeEventRequest(member))
    }

뭔가 이전코드보다 좀 덜 오염된 느낌이 들었다.

하지만 여전히 코드레벨에서 eventPublisher라는 공통된 아이가 반복적으로 선언되고 있는 것을 볼 수 있다,,,ㅜㅜㅜ

 

그래서 또 수정한 코드는 event작업만 담당하는 클래스를 두어 todoService에서 ApplicationEventPublisher에 관한 의존주입을 지웠다.

 

fun createTodoList(
    todoRequest: TodoRequest,
    username: String
): Long? {
   ...
    val issueEventRequest = eventProcessor.createIssue(member, todoRequest)
   ...
    eventProcessor.createReadMe(member)
    return todoListId

}

fun findTodoLists(todoDateRequest: TodoDateRequest, username: String): List<TodoResponse> {
    val member = memberReader.getMember(username)
    return todoReader.bringTodoLists(todoDateRequest.deadline, member)
}

fun finishTodoList(todoListId: Long, username: String) {
   ...
    eventProcessor.closeIssueWithReadMe(member, memberTodoList.issueNumber!!, ISSUE_CLOSED)
}

fun modifyTodoList(todoListId: Long, todoRequest: TodoRequest, username: String) {
    ...
    eventProcessor.updateIssueWithReadMe(
        member,
        todoList.issueNumber!!,
        todoRequest.toTodoCreate(member)
    )
}

fun removeTodoList(todoListId: Long, username: String) {
   ...
    eventProcessor.closeIssueWithReadMe(member, todoList.issueNumber!!, ISSUE_CLOSED)
}

@Transactional(readOnly = true)
fun calculateTodoList(
    todoCountRequest: TodoCountRequest,
    username: String
): List<TodoCountResponse> {
    val member = memberReader.getMember(username)
    return todoReader.countByTodoList(todoCountRequest, member)
}

fun unFinishedTodoList(todoListId: Long, username: String) {
    ...
    eventProcessor.closeIssueWithReadMe(member, findTodoList.issueNumber!!, ISSUE_OPEN)
}

코드가 정말 간결해진 것을 확인할 수 있었다...!

제가 생각하기에 이렇게 했을 때 오는 장점은 일단 재활용성이 좋다 라는 느낌을 받았다. 

예를 들어 이벤트가 다른 클래스에서 필요하다면 이것저것 설정을 할 필요없이 eventProcessor를 통해서 이벤트를 설정할 수 있다는 장점이 있다.

 

또 다음으로는 다른 개발자가 이 코드를 봤을 때 "이 메소드가 실행되면 이벤트도 함께 발행이 되는구나!" 라고 쉽게 파악이 가능할 거라고 생각이 들었다.

 

하지만 지금도 어찌됐든 이벤트 관련된 코드가 todoService에 오염이 되어있는 것을 볼 수 있다..ㅜㅜㅜ

그래서 생각한 방법은 바로 AOP를 통해서 하는 방법이다.

 

구현을 하기 전 일단 제가 생각한 방법에 대해 설명하겠습니다.

우선 저의 목표는 TodoService코드 안에서 이벤트처리관련된 코드가 사라지고, 뿐만 아니라 이벤트 관련 의존관계도 지우자! 이었습니다.

 

정말 간단하게 특정 어노테이션을 붙이면 이벤트가 발행되어서 기존의 이벤트코드의 역할을 대체하는 것 이었습니다..!!

 

@Target(AnnotationTarget.FUNCTION)
@Retention(AnnotationRetention.RUNTIME)
annotation class CreateEvent

우선 어노테이션을 선언한 후

@Aspect
@Component
class CreateIssueAspect (
    private val eventProcessor: EventProcessor,
    private val memberReader: MemberReader
){
    @Around("@annotation(CreateEvent)")
    fun aroundCreateIssue(joinPoint: ProceedingJoinPoint): Any? {
        val args = joinPoint.args
        val username = args[1] as String
        val member = memberReader.getMember(username)
        val todoRequest = args[0] as TodoRequest

        val issueEventRequest = eventProcessor.createIssue(member, todoRequest)

        val newArgs = args.copyOf()
        newArgs[2] = issueEventRequest

        return joinPoint.proceed(newArgs)
    }
}

아래와 같이 메소드를 생성해주는데 저는 create하는 이벤트에서는 총 2개의 어노테이션이 관여하게 됩니다.

 

그 이유는 todoList를 생성할 때 이슈를 생성한 후 생성된 이슈넘버를 repo에 저장한 후 readme파일을 작성하는 순으로 작성을 해두었기 때문에 Issue생성 관련 어노테이션은 Around, ReadME파일 작성 어노테이션은 After로 선언을 하였습니다.

 

그래서 위의 코드를 보면 joinPoint를 통해 메소드의 인자를 받아온 후 이벤트 처리를 진행하는데 newArgs[2]를 통해 생성된IssueNumber를 다시 원래 메소드로 넘기는 작업을 진행하였습니다.

 

(똑같이 ReadME생성관련 어노테이션도 똑같이 생성한 후 사용하였습니다!)

 

이렇게해서 

@CreateEvent
@ReadMeCreate
fun createTodoList(
    todoRequest: TodoRequest,
    username: String,
    issueEventRequest: IssueEventRequest? = null
): Long? {
    ...

    val issueNumber = issueEventRequest?.issueNumber?.get()
        ?: throw IllegalStateException("IssueEventRequest cannot be null")

 ...
}

위 코드처럼 issueEventRequest에서 IssueNumber를 받는 것을 수행하게 되었습니다.

그럼 이제 TodoList코드에는 이벤트 관련된 코드가 보이지 않고, 드디어 코드 오염에서 벗어나게 되었습니다 :)

 

@EventHandler
fun finishTodoList(todoListId: Long, username: String,state: String) {
    val memberTodoList = todoReader.findTodoList(todoListId)
    todoUpdater.doneTodoList(memberTodoList)
}

@UpdateEvent
fun modifyTodoList(todoListId: Long, todoRequest: TodoRequest, username: String) {
    val member = memberReader.getMember(username)
    val todoList = todoReader.findTodoList(todoListId)
    todoValidator.isWriter(todoListId, member)
    todoUpdater.update(todoList, todoRequest)
    issueValidator.isExist(todoList)
}
@EventHandler
fun removeTodoList(todoListId: Long, username: String, state: String) {
    val todoList = todoReader.findTodoList(todoListId)
    todoDeleter.delete(todoList)
}

다른 코드들도 완벽히 이벤트처리 관련된 코드들이 사라진 것을 확인할 수 있었습니다..!!!!!(오염에서 살아남자,,ㅎ)

 

이렇게 했을 때 제가 느낀 장점은 우선 한눈에 봐도 코드의 수가 확연히 줄었들었고, 어노테이션을 통해 이러한 이벤트가 발생하겠구나라는 것을 더 명확히 확인할 수 있다는 장점이 있다고 생각했습니다.

 

두번째 장점으로는 TodoService의 역할을 보다 더 분명하게 분리했다는 것이었습니다. 

물론 이전 코드도 실질적인 구현은 ~er를 통해 구현을 했지만 어찌됐든 todoService에 eventProcessor가 관여를 하고 있다는 점 이었고, 저는 이 부분이 책임이 많아졌다고 생각을 하였습니다..!

 

이렇게 코드수정을 단계별로 진행을 하면서 점점 간결해지는 코드와 역할을 명확하게 분리하면서 좀 더 나은 코드가 된 거 같다는 느낌을 받았습니다.ㅎㅎㅎ

 

이젠 스프링에서 제공해주는 이벤트는 동기식으로 처리가 되는데 이 부분을 무작정 비동기로 바꾸는 것이 아닌 상황에 맞게 수정을 해보겠습니다!!!

 

https://github.com/ToDeveloperDo/TDD-be

GitHub - ToDeveloperDo/TDD-be