[Spring] 안티 패턴과 Optional

책을 보다가 잘못된 코드를 짜고 있는 것을 발견해서 공부해보고자 한다.

안티패턴이란?

응용 프로그램을 설계할 때 발생하는 일반적인 문제 또는 상황을 위한 재사용하기 좋은 코드 패턴을 모아놓은 것을 디자인 패턴이라 부르는데, 이를 잘못 사용한다면 오히려 비효율적이거나 심각한 장애를 일으킨다고 한다. 이를 프로그래밍의 안티패턴이라고 부른다.

Optional

Optional 클래스의 주요 목적은 null 값을 다루는 방식을 개선하고 명시적으로 처리할 수 있도록 하는 것이다. 따라서 Optional을 사용하면 null에 의해 발생하는 예외를 방지하고 안정성 있는 코드를 만들 수 있는 것이다.

Optional의 안티패턴

Optional을 많이 사용해보지 않아서 찾아본 결과, 이러한 안티패턴이 있을 수 있다고 한다.

1. 필드로 사용금지
   : Optional은 Serializable를 구현하지 않기 때문에 필드로 사용하면 안된다.
2. isPresent()-get() 사용
   : 이 글을 쓰는 이유이자, 내가 실수했던 코드이다.
   if 문과 isPresent 메소드를 사용하여 조건을 처리했었는데, 이는 Optional로 처리하는 이유를 없애고 불필요하게 코드를 늘여뜨리는 결과를 낳는다.
3. Optional을 생성자나 메서드 파라메터로 사용금지
   : Optional을 생성자나 메서드 인자로 사용하면, 호출될때마다 Optional을 생성하기 때문에 리소스가 낭비된다.

이 문제들을 내 코드를 직접 확인하며 알아보자.

// 기존 회원 가입 DAO
@Transactional
	public void sign(UserDTO dto) throws Exception{
		try {
			Optional<Users> userOpt = userRepository.findByEmail(dto.getEmail());
			Users user = null;
			if(dto.getSnsAuth() == false) {
				if(userOpt.isPresent()) {	// (2번 문제)
					throw new Exception("존재하는 회원 이메일입니다.");
				}
				BCryptPasswordEncoder passwordEncoder = new BCryptPasswordEncoder();
				dto.setPw(passwordEncoder.encode(dto.getPw()));
				user = Users.toEntity(dto);
				WalletDTO walletDTO = new WalletDTO();
				walletDTO.setUser(user);
				walletRepository.save(Wallets.toEntity(walletDTO));
			}
			else {
				if(userOpt.isPresent()) {	// (2번 문제)
					user = userOpt.get();
					user.setSnsAuth(true);
					user.setName(dto.getName());
				}
				else {
					user = Users.toEntity(dto);
					WalletDTO walletDTO = new WalletDTO();
					walletDTO.setUser(user);
					walletRepository.save(Wallets.toEntity(walletDTO));
				}
			}
			userRepository.save(user);
		}catch(Exception e) {
			throw e;
		}
	}

코드에 적여있듯이, isPresent-get 패턴이 많이 보인다. 흐름 상 문제가 있는 부분이 있어 완벽하게 정리할 수는 없으나 최대한 Optional(?)하게 작성해보도록 하자.

ifPresent와 isPresent

회원 가입 여부를 판단할 때 isPresent로 분기하여 처리한 부분이 있다. isPresent보다는 ifPresent와 람다식을 함께 사용하여 한줄 코드로 작성하는 것이 일반적이다.
위 코드에서는 SNS 계정 여부를 판단해야 하기 때문에 불가능하지만 map-orElseGet을 사용하여 예외처리하는 방법도 있다.
++) Java9 버전 이상에서는 ifPresentOrElse()로 처리할 수도 있다.

user = userOpt.map(existingUser -> {	// map을 사용한 방식
    existingUser.setSnsAuth(true);
    existingUser.setName(dto.getName());
    return existingUser;
}).orElseGet(() -> {
    Users newUser = Users.toEntity(dto);
    WalletDTO walletDTO = new WalletDTO();
    walletDTO.setUser(newUser);
    walletRepository.save(Wallets.toEntity(walletDTO));
    return newUser;
});

userOpt.ifPresentOrElse(user -> {	// ifPresentOrElse를 사용한 방식
    user.setSnsAuth(true);
    user.setName(dto.getName());
}, () -> {
    user = Users.toEntity(dto);
    WalletDTO walletDTO = new WalletDTO();
    walletDTO.setUser(user);
    walletRepository.save(Wallets.toEntity(walletDTO));
});

orElse / orElseGet / orElseThrow

Optional을 사용할때 사용하는 메소드들이다. 파라미터로 람다식을 사용하여 null에 대한 추가 로직을 구현해준다.

• orElse와 orElseGet
  둘은 반드시 차이점을 알고 사용해야 한다. 우선 어떻게 구현돼있는지 확인해보자.

public final class Optional<T> {

		private final T value;
		
		public T orElse(T other) {
		    return value != null ? value : other;
		}
		
		public T orElseGet(Supplier<? extends T> supplier) {
		    return value != null ? value : supplier.get();
		}
}

둘은 모두 value가 null일 경우 대체값을 반환한다.
그러나 orElse는 이미 생성되었을 수 있는 대체값을 반환하며, orElseGet은 null을 확인한 이후에 대체값을 생성할 supplier를 호출하여 get()으로 반환한다.

이 둘의 차이점은 파라미터 안의 메소드가 실행되는 시점으로 확인할 수 있다.

optionalObj.orElse(printObj());	   // optional의 null과 상관없이 printObj를 항상 호출
optionalObj.orElseGet(() -> printObj());	// null 값을 확인하면 printObj를 호출하여 반환

위 코드에서 orElse는 optionalObj가 null인지 확인하기 전에 printObj를 실행하지만 orElseGet은 printObj를 실행하지 않는다.

만약 null이 아닐 경우에도 orElse의 경우 결과값을 사용하지 않더라도 printObj 메소드를 실행하기 때문에 불필요한 리소스를 소비할 수 있다. 이는 개발 의도와 다른 결과를 낳을 수 있어 치명적인 문제를 발생하는 안티패턴이 된다.
따라서 orElse보다는 orElseGet를 사용하도록 하자.

@Transactional
	public void sign(UserDTO dto) throws Exception{
		Optional<Users> userOpt = userRepository.findByEmail(dto.getEmail());
		Users user = null;
		if(!dto.getSnsAuth()) {
			userOpt.ifPresent(pre -> {	// 가입 여부 확인
				throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "존재하는 회원 이메일입니다.");
			});

			BCryptPasswordEncoder passwordEncoder = new BCryptPasswordEncoder();	// 비밀번호 해시화 및 유저 Entity 생성
			dto.setPw(passwordEncoder.encode(dto.getPw()));
			user = Users.toEntity(dto);

			WalletDTO walletDTO = new WalletDTO();	// 개인 지갑은 1:1이기 때문에 만들어주기
			walletDTO.setUser(user);
			walletRepository.save(Wallets.toEntity(walletDTO));
		}
		else {
			user = userOpt.map(existingUser -> {	// 가입된 유저가 있으면 SNS 계정으로 전환
				existingUser.setSnsAuth(true);
				existingUser.setName(dto.getName());
				return existingUser;
			}).orElseGet(() -> {	// 없으면 계정 생성
				Users newUser = Users.toEntity(dto);
				WalletDTO walletDTO = new WalletDTO();
				walletDTO.setUser(newUser);
				walletRepository.save(Wallets.toEntity(walletDTO));
				return newUser;
			});
		}
		userRepository.save(user);
	}

위는 수정된 코드이다. 구현 당시에 로직을 내가 짜지 않아서 뭔가 아쉽지만 최대한 Optional의 제작 의도에 맞게 설계해보았다.

다른 로직들 중에 ArrayList를 활용한 로직들이 있는데, Stream과 Optional의 메소드, 람다를 사용해서 전부 리뉴얼하고자 한다.
리뉴얼하면서 생기는 이슈는 추가로 포스팅해보고자 한다.

추가 개선 코드

회원가입 로직보다 개선된 점이 분명하게 보이는 코드를 추가로 게시하고자 한다.

// 기존 로그인 로직
public ResponseDTO login(String email, String pw, Boolean snsAuth) throws Exception{
		try {
			Optional<Users> userOpt = userRepository.findByEmail(email)
			if(userOpt.isEmpty()) {   // 안티패턴
				throw new Exception("존재하지 않는 회원입니다.");
			}
			else {
				Users user = userOpt.get();   // 안티패턴
				if(user.getSnsAuth() == false) {
					if(pw == null) {
						throw new Exception("입력 오류");
					}
					// 비밀번호 불일치 시 예외
					BCryptPasswordEncoder passwordEncoder = new BCryptPasswordEncoder();
					if(!passwordEncoder.matches(pw, user.getPw())) {
						throw new Exception("비밀번호 불일치");
					}
				}
				LoginDTO loginDTO = new LoginDTO(jwtUtil.generateToken(user.getId()));
				return new ResponseDTO(loginDTO, null);
			}
		}catch(Exception e) {
			throw e;
		}
	}

// 개선된 코드
public ResponseDTO login(String email, String pw) throws Exception{
		Users user = userRepository.findByEmail(email).orElseThrow(() ->   // 안티패턴 제거
				new ResponseStatusException(HttpStatus.BAD_REQUEST, "존재하지 않는 회원입니다."));

		if(!user.getSnsAuth()) {	// SNS 계정은 비밀번호를 요구하지 않음
			if(!passwordEncoder.matches(pw, user.getPw())) {	// passwordEncoder 재사용 (Autowired)
				throw new Exception("비밀번호 불일치");   // 비밀번호 불일치 시 예외
			}
		}
		
		LoginDTO loginDTO = new LoginDTO(jwtUtil.generateToken(user.getId()));
		return new ResponseDTO(loginDTO, null);
	}

확실히 가시적이고 간결한 코드로 리펙토링 되었다.

• orElseThrow로 null 값에 대한 예외처리
• 불필요한 try-catch문 삭제
• @Bean으로 등록하여 passwordEncoder 재사용 -> 클래스 자체가 상태를 가지지 않기에 Singleton 패턴을 적용하고자 함

추가적인 코드 개선 사항이 이루어질 것이며, 이후 코드는 아래의 notion에 게시할 예정이다.

Spring 코드 리펙터링

읽을 거리 및 참고 자료

• 프로그래밍 안티패턴
• Optional 안티패턴