[NestJS] 코드 리팩토링하기 - 응집도를 높이고 의존성을 명확하게

[NestJS] 코드 리팩토링하기 - 응집도를 높이고 의존성을 명확하게트러블슈팅2024. 1. 21. 21:59@mag1c

Table of Contents

728x90

서론

요즘 좋은 코드 라는 키워드에 대해

특히 변경과 재사용이 용이한, 높은 응집도와 낮은 결합 관계 에 대해 많이 생각하고 있다.

특히 기존 레거시를 모두다 걷어내기에는 시간적으로 애로사항이 있어

틈틈이 관련된 프로젝트에 들어갈 때, 해당 로직에 대한 레거시들을 최대한 바꾸려고 노력하고 있다.

[네이버클라우드 개발자 스토리] 좋은 코드란 무엇일까?🤔 #클린코드 이야기

📍 “좋은 코드를 짜야 한다”

medium.com

특히, 상품의 리뷰를 불러오는 함수를 수정해야 하는 일이 최근에 있었는데,

상품군 7~8개의 하위 상품에 대한 리뷰를 모두 다른 함수에서 불러오는 것을 보고 경악을 금치 못했다.

(급한 사항이라 판단되어 우선 프로덕션에 수정해서 반영한 뒤 구조를 수정하였다..)

나도 최근에 신규 프로젝트를 진행하면서

함수가 많아지고 코드가 길어짐에 따라

비즈니스 레이어에 있는 Validation관련 로직이 많아져서

Validation관련 함수들만 따로 클래스를 분리하여 Provider로 만들어주는 과정에서 있었던 일들을

간략하게나마 작성해 이 시기에 이런 고민을 했고, 후에 다른 백엔드 분들과 협업 시에 달라질 코드 스타일과 비교하고자,

마지막으로 지금의 나는 옳은 방향으로 가고 있었는지 미래에 확인해보기 위해 작성 해보려고 한다.

쭉 써내려간 코드

아래는, 출퇴 시간을 직접 조정할 수 있는 기능에 대한 코드이다.

//출결 등록, 변경 관련
@Injectable()
export class AttendenceService implements AttendanceServiceImpl {

    constructor(
        private readonly offDayPlanRepo: OffDayPlanRepository,
        private readonly offDayPlanCoreRepo: OffDayPlanCoreRepository,
        private readonly offDayPlanOutTimeRepo: OffDayPlanOutTimeRepository,
    ) { }
    
    
    //출퇴근 시간 직접 설정
    async customizeWorkTime(user: User, startTime: Date, endTime?: Date): Promise<CustomRes> {
    
    	//현재 시각의 Date, String
        const { today, todayString } = await dateDataSet();
        
        //오늘 날짜의 Row Check
        await this.existsOffDayPlanCheck(user, 'customIn', today, todayString);

        //부서별 코어 근무시간과 validation
        if (await this.registrationCustomVaildation(user.devision, startTime, endTime)) {
        	
            const result = await this.offDayPlanRepo.updateCustomWorkTime(user.id, startTime, endTime, now());

            if (result.affected) {
                let startMsg = (startTime) ? `${getFullDate(startTime)}` : '';
                let endMsg = (endTime) ? `${getFullDate(endTime)}` : '';
                const returnMsg = startMsg + ' ' + endMsg;
                return TimeRecordSuccess(`update complete : ${returnMsg}`);
            }
        }
    }
    
        
    //출퇴근 시간 직접 설정 시 근무가능시간 Validation
    // 1. 코어근무시간, 2. 근무 가능시간
    async registrationCustomVaildation(devision: string, startTime: Date, endTime?: Date): Promise<boolean> {

        const availableTimeVali = await this.coreTimeValidation(devision, startTime, endTime);
        const coreTimeVail = await this.availableWorkTimeValidation(devision, startTime, endTime);

        if (availableTimeVali && coreTimeVali) {
            return true;
        };

    }

	
    
    //부서별 코어 근무 시간과 비교해
    // 출퇴근 시간 등록 가능하면 바로, 안되면 승인 요청 요구.
    async coreTimeValidation(devision: string, startTime?: Date, endTime?: Date): Promise<boolean> {

        const { startTimeString, endTimeString } = await this.getCoreTimeByDevision(devision);

        if (startTime && startTimeString > getHoursAndMinutes(startTime)) {                        
            throw CoreTimeLangeException(`[request first] request first-1`);
        };
        if (endTime && endTimeString < getHoursAndMinutes(endTime)) {
            throw CoreTimeLangeException(`[request first] request first-2`);
        };

        return true;

    }


    //근무 가능 시간과 비교
    async availableWorkTimeValidation(devision: string, startTime?: Date, endTime?: Date): Promise<boolean> {

        //근무가능시간 시나리오가 나오면 작성 예정
        return true;

    }
    

}

해당 기능을 포함한 Attendence에 대한 비즈니스 로직, 벨리데이션 로직들을 쭉 써내려가다보니

벨리데이션을 분리하여 관리해주고 싶어졌다.

리팩토링

우선, 서비스의 비즈니스 로직에서 벨리데이션을 분리하여 응집도를 높여볼 수 있었다.

이는 벨리데이션의 조건이 변경되거나 추가될 때 등

벨리데이션을 관리하고 유지보수하는 데 더 용이하다.

//출결 등록, 변경 관련
@Injectable()
export class AttendanceValidationProvider {
    
        
    //출퇴근 시간 직접 설정 시 근무가능시간 Validation
    // 1. 코어근무시간, 2. 근무 가능시간
    async registrationCustomVaildation(devision: string, startTime: Date, endTime?: Date): Promise<boolean> {

        const availableTimeVali = await this.coreTimeValidation(devision, startTime, endTime);
        const coreTimeVail = await this.availableWorkTimeValidation(devision, startTime, endTime);

        if (availableTimeVali && coreTimeVali) {
            return true;
        };

    }

	
    
    //부서별 코어 근무 시간과 비교해
    // 출퇴근 시간 등록 가능하면 바로, 안되면 승인 요청 요구.
    async coreTimeValidation(devision: string, startTime?: Date, endTime?: Date): Promise<boolean> {

        const { startTimeString, endTimeString } = await this.getCoreTimeByDevision(devision);

        if (startTime && startTimeString > getHoursAndMinutes(startTime)) {                        
            throw CoreTimeLangeException(`[request first] request first-1`);
        };
        if (endTime && endTimeString < getHoursAndMinutes(endTime)) {
            throw CoreTimeLangeException(`[request first] request first-2`);
        };

        return true;

    }


    //근무 가능 시간과 비교
    async availableWorkTimeValidation(devision: string, startTime?: Date, endTime?: Date): Promise<boolean> {

        //근무가능시간 시나리오가 나오면 작성 예정
        return true;

    }
    

}

모듈의 Provider에 ValidationProvider을 추가하고, 서비스로 DI하여 사용하면 되겠다.

고 생각하였으나 문제가 발생했다.

Validation Provider의 코드를 자세히 보면 아래와 같은 코드가 있다.

const { startTimeString, endTimeString } = await this.getCoreTimeByDevision(devision);

위의 코드는, 사내 부서별 코어 근무시간과, 근무 가능 시간을 불러오는 코드로, 서비스단에 구현되어 있으며

현재는 커스텀 리파지토리를 구현하여 findOneByDevision(devision)을 호출하게 되어있다.

/**
 * 부서별 코어 근무시간, 근무 가능시간 조회
 * @param devision 
 * @returns 
 */
async getCoreTimeByDevision(devision: string): Promise<OffDayPlanCoreEntity> {

    const entity = await this.offDayPlanCoreRepo.findOne({ where: { devision: devision } });
    if (!entity) {
        throw CoreTimeRecordNotFoundException([Not Found] `${devision} row data not found`);
    };

    return entity;
}

Validation Provider에서 다시 서비스단에 의존을 하게 된다면,

Validation Provider가 단순 출결 서비스의 벨리데이션을 담당하는 모듈이 아니라 서비스와 동일한 수준의 모듈이 되어버린다. DIP를 위반하게 되는 것이다. (상위 수준의 모듈은 하위 수준의 모듈에 의존해서는 안된다)

또한 직접 의존하게 될 경우 결합도가 상승하여 로직이 변경될 경우, 서로에게 영향을 끼치게 된다.

최악의 경우 코드를 모두 수정해야할 수도 있다.

또한 Validation 클래스에서 직접 Repository를 호출하는 것 또한 DIP를 위배하고,

Validation만 수행하게 하려고 역할을 분리하여 클래스를 설계했는데, 이에 위배된다고 생각했다.

이런 저런 생각들을 통해 아래처럼 코드를 최종 변경할 수 있었다.

//부서별 코어 근무 시간과 비교해
// 출퇴근 시간 등록 가능하면 바로, 안되면 승인 요청 요구.
async coreTimeValidation(entity: CoreTimeEntity, startTime?: Date, endTime?: Date): Promise<boolean> {

    const { startTimeString, endTimeString } = entity;

    if (startTime && startTimeString > getHoursAndMinutes(startTime)) {                        
        throw CoreTimeLangeException(`[request first] request first-1`);
    };
    if (endTime && endTimeString < getHoursAndMinutes(endTime)) {
        throw CoreTimeLangeException(`[request first] request first-2`);
    };

    return true;

}

1. coreTimeValidation라는 네이밍에 맞게 코어 근무시간만 Validation하였으며

2. entity를 파라미터로 받았다.

이제 이 Validation 클래스는 이름에 맞게, 출퇴근의 Validation만 담당하게 될 것이며

Validation의 조건이 변경될 경우 해당 Validation만 변경해주거나, 파라미터만 수정해준다면 올바르게 동작할 것이다.

02.16 추가

감사하게도 댓글의 좋은 피드백을 받아, 한 번의 리팩토링을 더 거칠 수 있게 되었다.

1. 사용하지 않는 변수 제거

2. 함수명이 명확한 의미를 가지게 변경,

3. 함수를 boolean타입으로 리턴받아서 추가적으로 핸들링하는 것이 없기 때문에 void

(단순 에러 or 통과)

4. validateCoreTime만 봐도 어떤 부분들을 검증하는지 명확하게 볼 수 있게 내부 함수로 변경

특히 2,4번은 간과하고 있던 부분이라고 생각했다.

(다시한번 좋은 피드백을 제공해 주셔서 감사하다는 말씀을 전합니다.)

validateCoreTime(entity: CoreTimeEntity, startTime?: Date, endTime?: Date): void {

    const { startTimeString, endTimeString } = entity;

    if (startTime) {                        
    
    	this.validateStartCoreTime(startTimeString, startTime);
    
    };
    
    if (endTime) {
    
    	this.validateEndCoreTime(endTimeString, endTime);
    
    };
    
}

private validateStartCoreTime(startTimeString: string, startTime: Date): void {

	if (startTimeString > getHoursAndMinutes(startTime)) {
    
    	throw CoreTimeLangeException('[request first] request first-1');
    
    };

};
    

private validateEndCoreTime(endTimeString: string, endTime: Date): void {

	if (endTimeString > getHoursAndMinutes(endTime)) {
    
    	throw CoreTimeLangeException('[request first] request first-2');
    
    };

};

상위 함수도 변경할 수 있었다.

1. 상위 모듈인 서비스에서 entity를 들고오고

2. 네이밍도 명확하게 변경시켜주었다.

3. 또한 하위의 각 함수들이 무슨 역할을 하는지도 명확히 전달될 수 있게 변경해주었고

4. void타입의 함수들이기 때문에 함수의 로직도 변경시켜주었다.

async validateCustomWorkTime(entity: CoreTimeEntity, startTime: Date, endTime?: Date): Promise<void> {

    await this.validateCoreTime(entity, startTime, endTime);
    await this.validateAvailableWorkTime(entity, startTime, endTime);

}