[중고거래장터 - 6] setter 대신 빌더 패턴 사용 / Controller와 Service 계층의 역할 분리

기존 작업물에 대한 리팩토링을 했다. 

 

- setter 메소드 대신 빌더 패턴 사용하기

- jUnit 테스트 코드 리팩토링

- 도메인 클래스 lombok 설정 수정

- 구현 세부 내용 숨기기

 

setter 메소드 대신 빌더 패턴 사용하기

 

아래 코드는 사용자의 비밀번호 변경 요청을 처리하는 Controller 클래스의 메소드이다.

이 코드에서 가장 먼저 수정해야 할 부분은 setter 메소드를 사용해서 값을 변경하는 부분이라고 판단했다. setter 메소드를 통해 유연하게 값을 받아서 넘겨주는 것이 편하게 느껴질 수 있다. 하지만 이런 로직은 불안하다.

 

• 수정자 메소드(setter)의 사용은 불필요한 데이터 변경을 유발한다.
• 이는 객체의 일관성을 유지하기 어렵게 만든다. 또한 의도치 않은 변경이 발생할 가능성이 항상 열려있다.
• 객체는 확장에는 열려있고, 수정에는 닫혀 있어야 한다는 개방-폐쇄의 원칙에 어긋난다.

 

추가로 Controller 클래스는 사용자의 요청에 대한 데이터 전달을 중심으로 구현해야 하는 클래스라고 생각한다. 그런데 아래 코드처럼 수정자 메소드를 사용해서 값을 수정하는 것은 옳지 않다고 생각했다. 값이 수정되는 구현 과정을 굳이 노출시킬 필요는 없으니 Service 레이어로 이동시키는 것이 옳다고 판단했다. 게다가 수정자 메소드의 사용은 의도치 않은 데이터 변경의 가능성을 열어둔다. 

 

UserController.java

@PostMapping(value = "/help/pwd/{userId}", produces = "application/text; charset=UTF-8")
public ResponseEntity<String> pwdChange(@RequestBody UserVO user, @PathVariable("userId") String userId) {
	try {
		user.setUserId(userId);
		service.changeUserPwd(user);
	} catch (Exception e) {
		return new ResponseEntity<String>("에러 발생. 다시 요청해주세요." ,HttpStatus.BAD_REQUEST);
	}
	return new ResponseEntity<String>("비밀번호 변경 완료" ,HttpStatus.OK);
}

 

수정자 메소드를 대체할 수 있는 방법을 찾아보다가 빌더 패턴을 통한 리팩토링 방법을 발견했다. 직접 코드를 통해 구현할 수 있지만, lombok을 사용하면 손쉽게 사용할 수 있다. 

 

아래 코드는 빌더 패턴을 적용한 Service 클래스이다. 새롭게 변경된 비밀번호를 암호화해서 전달하는 일련의 과정을 보여주고 있다. 

 

UserServiceImpl.java

@Override
public void changeUserPwd(UserVO user) {
		
	String encodedPwd = PasswordEncryptor.encrypt(user.getUserPwd());

	UserVO encryptedUser = UserVO.builder()
			.userId(user.getUserId())
			.userPwd(encodedPwd)
			.build();
		
	mapper.changeUserPwd(encryptedUser);
}

 

빌더 패턴을 사용을 통해 개선된 점은 다음과 같다.

 

• 객체의 불변성 확보가 좀 더 수월해졌다. 위 코드처럼 메소드 내부에서 필요에 따라 독립된 객체를 생성해서 처리하면 생성된 객체의 활동 범위는 메소드 내부로 한정된다. 즉 외부의 간섭을 전혀 받지 않기 때문에(의존도X) 객체의 불변성을 확보했다고 할 수 있지 않을까?
• 기존의 생성자를 통한 객체 생성 방식과 비교해서 간편하게 생성할 수 있고, 가독성이 좋아서 코드가 깔끔해졌다.

 

도메인 클래스 lombok 설정 수정

 

"도메인 클래스 설정부터 제대로 해야 객체의 불변성이 보장되지 않을까?"

"나는 의도에 맞게 도메인 클래스 설정을 제대로 했는가?"

수정자 메소드를 빌더 패턴을 통해 리팩토링하는 과정에서 떠오른 생각이다. 먼저 기존의 도메인 클래스를 보자.

 

lombok의 @Data 어노테이션을 사용하고 있다. @Getter, @Setter, @RequiredArgsConstructor, @ToString, @EqualsAndHashCode까지 한꺼번에 설정해주는 어노테이션인데, 그만큼 편하지만, 제공하는 기능이 모두 필요하지 않다면(특히 수정자 메소드) 제한적인 기능을 제공하는 어노테이션을 사용해서 불필요한 실수를 방지하는 방법이 더 좋다고 판단했다.

 

@Data
public class UserVO { ... }

 

수정된 도메인 클래스는 다음과 같다.

 

@Getter
@Builder
@AllArgsConstructor
@NoArgsConstructor
public class UserVO { ... }

 

@Getter	각 필드에 대한 접근자 메소드 생성.
@Builder	해당 클래스의 빌더 패턴을 자동으로 생성해준다.
@NoArgsConstructor(force = true)	파라미터가 없는 기본 생성자를 만든다. 그리고 (force = true) 설정을 통해 final 필드를 기본값(0, false, null)으로 초기화 한다.
@RequiredArgsConstructor	@NonNull 어노테이션 또는 final 키워드가 있는 필드를 파라미터로 가지는 생성자를 만든다.

 

@Getter 메소드만 사용할 수 있도록 하여 수정자 메소드의 사용을 원천적으로 막았다. 만약 객체 내부의 값에 수정이 필요하다면 빌더 패턴을 통해 독립된 새로운 객체를 생성해서 구현하면 된다.

 

@RequiredArgsConstructor는 클래스 필드를 private final로 설정했고, final 필드를 가지는 생성자를 만들기 위해 사용했다. 이렇게 하면 안전하게 불변값을 가지는 객체를 생성할 수 있다고 판단했다.

 

@NoArgsConstructor를 선언한 이유는 MyBatis가 자동으로 객체를 생성할 때 기본적으로 파라미터가 없는 생성자가 필요하기 때문이다. 

 

Controller와 Service 계층의 역할 분리

 

아래 코드는 사용자의 요청에 따라 사용자 이메일로 인증 번호 전송 요청을 처리하는 Controller와 Service 계층의 메소드이다.

 

UserController.java / Controller 계층

@GetMapping(value = "/help/pwd/email", produces = "application/text; charset=UTF-8")
public ResponseEntity<String> sendCertEmail(@RequestParam("userId") String userId, @RequestParam("userEmail") String userEmail) {
		
	int cnt = service.checkUserIdEmail(userId, userEmail);
	if (cnt == 0) {
		return new ResponseEntity<String>("잘못된 아이디 또는 이메일 주소" ,HttpStatus.CONFLICT);
	}
    
	Random random = new Random();
       int certNum = random.nextInt(888888) + 111111;
	   String code = "";
	    
	 try {
	    EmailUtils.sendEmail(userEmail, certNum);
	    code = Integer.toString(certNum);
	    log.warn("code ======== " + code);
	 } catch (Exception e) {
	    return new ResponseEntity<String>("에러 발생. 다시 요청해주세요." ,HttpStatus.BAD_REQUEST);
	 }
	return new ResponseEntity<String>(code ,HttpStatus.OK);
}

 

UserServiceImpl.java / Service 계층

@Override
	public int checkUserIdEmail(String userId, String userEmail) {
		return mapper.checkUserIdEmail(userId, userEmail);
}

 

내가 이상하다고 생각했던 부분은, Controller 계층의 checkUserIdEmail() 메소드이다.

 

int cnt = service.checkUserIdEmail(userId, userEmail);
	if (cnt == 0) {
		return new ResponseEntity<String>("잘못된 아이디 또는 이메일 주소" ,HttpStatus.CONFLICT);
}

 

checkUserIdEmail() 메소드는 이메일 인증 번호 전송 과정에서 필요한 사용자 아이디와 사용자 이메일 주소가 실제로 데이터베이스에 있는 값인지를 검증하는 메소드이다. 위 코드를 보면 메소드가 어떤 과정을 통해 처리되는지 쉽게 파악할 수 있다. 문제는 이 메소드가 Controller 계층에 위치하는 것이 어색하다는 것이다. Controller 계층의 역할은 데이터의 전달 이라고 생각한다.

 

View 계층에서 사용자의 요청이 발생하면 Controller 계층에서 응답을 받은 후 Service 계층에게 이런 데이터 처리 요청이 넘어왔으니 처리 해달라고 요청한다. 그러면 Service 계층에서는 데이터베이스와 연계해서 동작하는 Repository 계층을 통해 구현된 비즈니스 로직을 수행해서 그 결과 데이터를 Controller에게 응답한다. 

 

이런 흐름을 생각했을 때 checkUserIdEmail() 메소드가 Controller 계층에 위치하는 것이 어색하다고 판단했다.

Controller 계층은 데이터가 어떻게 처리되는지 관심을 가질 필요가 없다는 것이 나의 생각이다. 데이터 처리에 대한 관심은 Service 계층과 Repository 계층에서 하면 된다.

 

그리고 checkUserIdEmail() 메소드 이후에 인증번호 난수를 생성하는 코드가 있는데, 코드 내부에서 난수를 생성하여 변수에 할당하고 있다. 

 

Random random = new Random();
int certNum = random.nextInt(888888) + 111111;

 

구현 과정을 노출하지 않고 캡슐화 하여 메소드를 통해 호출하는 방식을 취하면 코드가 더욱 깔끔해질 것이다.

 

수정 결과는 아래와 같다.

 

UserController.java

@GetMapping(value = "/help/pwd/email", produces = "application/text; charset=UTF-8")
	public ResponseEntity<String> sendCertEmail(@RequestParam("userId") String userId, @RequestParam("userEmail") String userEmail) {
		
	    String certNum = EmailUtils.getCertNum();
	    
	    try {
	    	service.isValidIdAndEmail(userId, userEmail);
	    	
	    	EmailUtils.sendEmail(userEmail, certNum);
	    	code = Integer.toString(certNum);
	    	log.warn("code ======== " + code);
	    } catch (Exception e) {
	    	return new ResponseEntity<String>("에러 발생. 다시 요청해주세요." ,HttpStatus.BAD_REQUEST);
	    }
		return new ResponseEntity<String>(code ,HttpStatus.OK);
}

 

UserServiceImpl.java

@Override
public void isValidIdAndEmail(String userId, String userEmail) {
	if (userMapper.isValidIdAndEmail(userId, userEmail).equals(INVALID)) {
		throw new UserNotExistsException(NOT_EXISTS_MSG);
	}
}

 

EmailUtils.java

public static String getCertNum() {
	Random random = new Random();
	int randomNum = random.nextInt(888888) + 111111;
	return Integer.toString(randomNum);
}

 

기존의 EmailUtils 클래스에 인증번호 난수 생성 메소드를 추가해서 클래스로부터 바로 호출할 수 있도록 구현했다. 또한 기존에 int 형태로 생성된 인증번호 값을 받은 후 String 형태로 수정하는 과정을 거치는데, 이 또한 캡슐화된 메소드에서 처리하도록 했다. 덕분에 코드가 깔끔해졌다.

그리고 checkUserIdEmail() 메소드에 대한 처리를 Service 계층이 처리하도록 했다. Controller와 Service 계층의 역할이 좀 더 명확히 분리된 것 같다.

 

Service 계층을 보면 checkUserIdEmail() 메소드가 isValidIdAndEmail() 메소드로 변경된 것을 확인할 수 있다. 기존에 숫자로 결과를 처리하던 방식에서 'TRUE', 'FALSE'와 같은 문자열 리턴 방식으로 쿼리문을 수정했다. Oracle을 사용하고 있기 때문에 case ~ when ~ else 구문을 사용했다. 쿼리는 프로젝트 깃허브 링크를 통해 확인할 수 있다. 그리고 개인적인 생각이지만, 이 과정을 통해 메소드 이름 선정의 중요성을 느꼈다. checkUserIdEmail() 메소드보단 isValidIdAndEmail() 메소드 이름이 해당 메소드가 수행하는 기능을 따져봤을 때 더 적절하다고 판단했다. 프로젝트를 진행하면서 더 좋은 이름을 발견하면 또 수정해야겠다.

 

jUnit 테스트 코드 리팩토링 작업도 진행했지만 특별히 언급할 부분이 없어서 생략한다.

 

후기

처음 프로젝트를 시작했을 때의 느낌은 마치 바람이 부는 방향을 따라 흘러가는 배를 타는 느낌이었다. 프로그래밍 언어와 프레임워크라는 거대한 바닷속에서 갈피를 잡지 못했다.

그런데 계속 부족한 부분을 공부하고, 왜 이렇게 구현해야 하는가에 대한 질문을 하면서 프로젝트를 진행하다 보니 점차 내가 원하는 방향으로 배를 조종하고 있다는 느낌이 든다. 아직 갈 길이 멀다. 이쯤 하면 괜찮겠지 싶다가도 다시 뒤돌아보면 부족한 부분이 많이 보인다. 그래도 그만큼 발전했기 때문에 어느 부분이 부족한지 알 수 있는 것이라고 생각한다. 계속 노력해서 부족한 부분을 점점 줄여나가자.