이민재

이민재

공부/일상 기록

리팩토링 - Facade 패턴을 적용해 Service 레이어 책임 줄이기

9 분 소요

서론

얼마전 현직자분과 멘토링을 진행할 기회가 생겨 한시간 반가량의 멘토링을 받았는데, 감사하게도 간단한 프로젝트 코드 리뷰도 진행해주셨다.

사실 공부를 하며 항상 부족함을 느꼈던 부분이 바로 이런부분이었는데, 전문가가 되기 위한 반복작업 중 가장 중요한 마지막 작업인 피드백이 어렵다는 것이었다. 어쨌든 여러가지 좋은 조언 중 가장 와닿았던게 프로젝트 코드에 대한 내용이어서 본격적으로 리팩토링을 진행해보려한다.

특히 디자인패턴과 객체지향, 추상화에 대해서 어느정도 알고 있다고 생각했는데, 실제로 코드를 작성하는데에 있어서는 제대로 적용이 안되어 있다는 생각이 가장 크게 들었다.

그래서 디자인 패턴도 적용해보고 클래스간의 과도한 의존성과 너무 많은 책임을 정말 세세히 분리하고 기능들을 추상화 해보며, 리팩토링을 진행해보려한다.

[링크] 리팩토링 할 프로젝트 주소

문제

문제 - 서비스 레이어에 너무 많은 책임이 있다

가장 먼저 지적받은 부분이기도 하고, 가장 크게 와닿기도 한 부분이다.

현재 내 서비스 레이어는 중 하나의 코드는 다음과 같은데,

[링크] VolunteerService 코드

보면 벌써 Repository가 다섯개씩 있고 코드가 하나의 클래스에 너무 많다. 사실 코드를 작성하면서 서비스에만 모든 코드가 다 있는데 굳이 이걸 컨트롤러 모델 서비스 구조로 하는게 의미가 있나? 싶은 생각이 들기도 했지만 거기서 생각이 멈추어서 그냥 남들이 하는대로 했었다.

이렇게 하면 레포지토리가 하나 새로 생기거나 기능이 추가될때마다 코드를 변경해야 하기 때문에 객체 지향스럽지 못하고, 이로인해 테스트코드까지 굉장히 무겁고 복잡하게 돌아간다.

실제로 내 테스트 코드는 다음과 같은데,

[링크] 테스트 코드

코드의 양이 굉장히 많고, 유닛테스트가 아니라 통합테스트로만 작성되어있다.

그래서 이번 게시물에서는 서비스레이어의 의존성을 줄여주고, 클래스단위로 더 나누는 방법을 찾아보며, 이를 적용해보고자 한다.

네이밍 바꾸기

우선 변수명과 클래스 명을 대대적으로 좀 바꾸려고 한다.

사실 변수명이 중요하다는 걸 알면서도 정말 깊게 고민하는데에 시간을 쓰지는 않은 것 같다.

Apply

Apply 클래스의 엔티티의 코드는 다음과 같다.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28

@Getter
@NoArgsConstructor
@Entity
public class Apply extends BaseTimeEntity {
    @Id
    @GeneratedValue(strategy = GenerationType.IDENTITY)
    private Long id;

    @ManyToOne
    @JoinColumn
    private User user;

    @ManyToOne
    @JoinColumn
    private Volunteer volunteer;

    @Builder
    public Apply(User user, Volunteer volunteer) {
        this.user = user;
        this.volunteer = volunteer;
    }

    public ApplyResponseDto toDto() {
        return ApplyResponseDto.builder()
                .entity(this)
                .build();
    }
}

보다싶이 봉사와 신청자의 정보를 담고있는 중간테이블인데, 이름이 Apply라는 동사가 되어버리니까 이상하기도 하고, 뒤에서 주로 여러개를 불러 오는데 자주 사용했는데, applies로 하지 않고 applyList로 이름을 붙였다.

하지만 applyList로 하면 이게 반드시 List인 변수여야 함으로, 후에 set으로 바뀐다거나하면 이름을 또 바꿔줘야 한다. 따라서 복수인 applies를 쓰는게 더 좋다고 한다.

사실 뒤에 s를 붙이는게 가독성이 안좋은것 같아서 List를 붙인 거였는데, 혹여 내가 set인데도 복수형으로 List를 사용한다면 사용하는 사람 입장에서 굉장히 혼란이 많이 올 것 같다는 생각이 들었다.

근데 애초에 그에 앞서서 Apply자체가 동사이기 때문에, s를 붙여 복수를 표기하는것 부터가 이상하다. 문법에 너무 안맞다.

따라서 Apply가 아니라 신청서라는 뜻의 Application으로 바꿔주기로 했다. 유저의 이름과 봉사이름이 들어가는 클래스이니 의미 상 더 알맞은것 같다.

Apply -> Application

ApplyList -> Applications

AppplyService -> ApplicantService

VolunteerService 책임 분리하기

이름부터 VolunteerService라는 이름으로, 모든 봉사 관련 비즈니스 로직을 여기 하나에서만 처리하게 하고있다. 이를 우선 분리하여 여러개의 클래스에 나누어 하나의 책임씩만 갖게 해야한다.

우선 VolunteerService가 가진 기능을 정리해보면 다음과 같다.

  1. 모든 봉사 조회
  2. id로 특정 봉사 조회
  3. 봉사 저장
  4. 봉사 수정
  5. 봉사 삭제
  6. 좋아요 누르기
  7. 봉사 신청
  8. 댓글 작성

이렇게 도합 8개의 기능이나 서비스 클래스 하나에서 담당하고 있었다.

이를 추상화하고 객체로 나누기 위해서 조금 더 생각해보기로 했다.

우선 봉사라고 뭉뜽그려 놓은 이름이 문제라는 생각이 들었다. 정확히는 ‘봉사 게시글’이고, 좋아요에 대한 내용과 댓글에 대한 내용도 다른 곳에 있어야 하지않나 싶다. 또, 같은 봉사게시글이더라도 봉사를 등록하고 수정, 삭제하는 것은 운영진의 기능이고, 봉사를 신청하고 댓글을 작성하는건 유저의 역할인데, 이 역할도 세심하게 나눠주지 않은 것 같다는 생각이 들었다.

그렇게 해주지 않은 결과로, VolunteerService 하나의 다른 도메인의 레포지토리가 5개가 모여있다. 이상적이게 객체를 분리했다면 VolunteerRepository 하나만 있어야한다.

image1

그래서 우선 가장 중점적으로 둘 것은 Repository가 Service당 하나만 있게하여, Repository의 의존성을 줄여보도록 할 것 이다.

현재 의존성 다이어그램

image1

기존에 이미 VolunteerRepository만을 사용하던 메소드를 제외하고, 다른 Repository를 의존하던 함수들을 추려보면 다음과 같다.

  1. id로 특정 봉사 조회
  2. 좋아요 누르기
  3. 봉사 신청
  4. 댓글 작성

그럼 이 메소드들을 리팩토링 해보자

VolunteerService / findById

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36

@Transactional(readOnly = true)
public VolunteerArticleResponseDto findById(Long id) {
	    Volunteer entity = volunteerRepository.findById(id).orElseThrow(NoExistVolunteerException::new);
	  // 기준으로 둘 entity 불러오기
	
	    List<Apply> applications = applicationRepository.findByVolunteer(entity);
	  
	    List<User> applyUser= applies.stream()
	            .map(Apply::toDto)
	            .map(ApplyResponseDto::getUser)
	            .collect(Collectors.toList());
	   // 1번 기능
	   
	    List<Favorite> favorites = favoriteRepository.findByVolunteer(entity);
	
	    List<User> favoriteUsers = favorites.stream()
	            .map(Favorite::toDto)
	            .map(FavoriteResponseDto::getUser)
	            .collect(Collectors.toList());
	    // 2번 기능
	   
	    List<Comment> commentList = commentRepository.findByVolunteer(entity);
	     
	    List<CommentResponseDto> commentListDto = commentList.stream()
	            .map(Comment::toDto)
	            .collect(Collectors.toList());
	    // 3번 기능
	
	    return VolunteerArticleResponseDto.builder()
	            .entity(entity)
	            .pictures(entity.detailPicture())
	            .applicants(applications)
	            .favoriteUsers(favoriteUsers)
	            .comments(commentListDto)
	            .build();
}

해당 메서드는 봉사 게시물을 특정 아이디로 불러오는 메서드이다.

대략 문단 별로 1번 기능, 2번 기능, 3번 기능으로 생각해보면 기능은 다음과 같다

  • 1번 기능

해당 봉사 신청자들을 불러와 이를 Dto에 담은뒤, Apply의 필드인 user의 정보로 변환하여 리스트로 반환한다.

  • 2번 기능

해당 봉사에 좋아요를 누른 사람들을 불러와 이를 Dto에 담은뒤, Favorite의 필드인 user의 정보로 변환하여 리스트로 반환한다.

  • 3번 기능
    해당 봉사에 댓글 정보를 가져와 Dto에 담아 리스트로 반환한다.

이렇게 보니, 그렇게 리팩토링이 어렵지 않아보인다. 우선 ApplyListService, FavoriteService, CommentService를 나누어서 저 위의 코드를 옮겨주도록 하자.

VolunteerService

1
2
3
4
5
6
7
8
9
10
11

@RequiredArgsConstructor
@Service
public class VolunteerService {
        private final VolunteerRepository volunteerRepository;

	    @Transactional(readOnly = true)
	    public Volunteer getVolunteerById (Long id) {
	        Volunteer entity = volunteerRepository.findById(id).orElseThrow(NoExistUserException::new);
	        return entity;
	    }
}

ApplicationService

1
2
3
4
5
6
7
8
9
10
11

@RequiredArgsConstructor
@Service
public class ApplicationService {
    	private final ApplicationRepository applicationRepository;

        public List<Application> getApplicationsByVolunteer(Volunteer entity) {
        List<Application> applications = applicationRepository.findByVolunteer(entity);

        return applications;
    }
}

FavoriteService

1
2
3
4
5
6
7
8
9
10
11

@RequiredArgsConstructor
@Service
public class FavoriteService {
    private final FavoriteRepository favoriteRepository;

    public List<Favorite> getFavoriteByVolunteer(Volunteer entity) {
        List<Favorite> favorites = favoriteRepository.findByVolunteer(entity);

        return favorites;
    }
}

CommentService

1
2
3
4
5
6
7
8
9
10
11
12

@RequiredArgsConstructor
@Service
public class CommentService {
    private final CommentRepository commentRepository;

    public List<Comment> getCommentsByVolunteer(Volunteer entity) {
        List<Comment> commentList = commentRepository.findByVolunteer(entity);

        return commentList;
    }
}

우선 기능이 정확하게 돌아가는건 미뤄두고, 러프하게 위에서 findById가 진행하던일을 각자의 Service 클래스를 만들어 세부분으로 나눠보았다.

이름도 기존에 findById등으로 너무 레포지토리스러운 함수 네이밍이었는데, get이 라는 이름을 통해서 리팩토링 해줌으로써 찾는것이 아니라 객체로부터 받는 느낌을 주게 이름을 변경해 보았다.

그런데 이렇게 하면 기존의 기능이 그대로 구현이 될까?

기존에는 Repository가 하나의 서비스에 몰려있었는데, 이들이 각각 떨어져 하나의 레포지토리만 갖도록 서비스가 분리 되었다면, 어떻게 Repository로 불러온 정보를 가공하고 조합하여 컨트롤러에 건네 줄 수 있을까?

이를 해결하기 위해 고민하던 중 Facade 패턴에 대해 알게 되었다.

Facade 패턴이란?

Facade란 건물의 외관이라는 뜻이다.

이름에서 어느정도 이 패턴을 유추할 수 있는데, 건물의 외관처럼 밖에서 보는 사람은 내부를 볼수 없듯이, 코드의 외관만 보고 요청을 하게 함으로써 내부 동작을 숨기는 방식이다.

위에서 나눈 기능들을 VolunteerFacade라는 클래스를 통해 클라이언트에게 제공함으로써, 클라이언트는 CommentService라던가 ApplicationService라던가 등의 세부 동작을 아예 몰라도 그냥 VolunteerFacade 객체에만 요청하면 된다.

따라서 Facade 클래스를 다음과 같이 구현해보았다.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34

@RequiredArgsConstructor
@Service
public class VolunteerFacade {
    private final VolunteerService volunteerService;
    private final FavoriteService favoriteService;
    private final CommentService commentService;
    private final ApplicationService applicationService;

    @Transactional(readOnly = true)
    public VolunteerArticleResponseDto getVolunteerArticleById(Long id) {

        Volunteer volunteerEntity = volunteerService.getVolunteerById(id);

        List<Application> applyList = applicationService.getApplicationsByVolunteer(volunteerEntity);
        List<Favorite> favoriteList = favoriteService.getFavoriteByVolunteer(volunteerEntity);
        List<Comment> commentList = commentService.getCommentsByVolunteer(volunteerEntity);

        List<User> applyUserList = applyList.stream()
                .map(Application::getUser)
                .collect(Collectors.toList());

        List<User> favoriteUserList = favoriteList.stream()
                .map(Favorite::getUser)
                .collect(Collectors.toList());

        return VolunteerArticleResponseDto.builder()
                .entity(volunteerEntity)
                .pictures(volunteerEntity.detailPicture())
                .applicants(applyUserList)
                .favoriteUsers(favoriteUserList)
                .comments(commentList)
                .build();
    }
}

이렇게 하면, Service마다 하나의 Repository만 갖게 함으로써 의존성을 줄이고, 더 상위의 개념으로 묶음으로써 사용자도 여러 내부구조를 알 필요없이 봉사 관련된 요청이라면 VolunteerFacade에게 맞는 요청을 보내기만 하면된다.

뿐만아니라, 만약에 나중에 정기 봉사만을 담당하는 RegularVolunteerService가 생겼다고 해도, VolunteerFacade에서 부품을 갈아끼기만 하면 된다.

Facade를 사용한 의존성 다이어그램

image1

따라서 위와 같은 의존성을 가지게 함으로써 컨트롤러 자체는 Facade에만 의존하고, VolunteerService하나에만 의존하던 결합성이 여러 Service로 나누어질 것 이다.

리팩토링 해보기

VolunteerService.isApplyVolunteer()

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28

  @Transactional
  public boolean isApplyVolunteer(Long volunteerId, UserEmailRequestDto userEmailRequestDto) {
	        User user = userRepository.findByEmail(userEmailRequestDto.getEmail()).orElseThrow(NoExistUserException::new);
	
	        Volunteer volunteer = volunteerRepository.findById(volunteerId).orElseThrow(NoExistVolunteerException::new);
	
	        List<User> userList = volunteer.getApplications().stream()
	                .map(Application::toDto)
	                .map(ApplyResponseDto::getUser)
	                .collect(Collectors.toList());
	
	        if (!userList.contains(user)) {
	
	            Application applyList = Application.builder()
	                    .volunteer(volunteer)
	                    .user(user)
	                    .build();
	
	            applicationRepository.save(applyList);
	            updateCurrentUser(volunteer);
	            return true;
	        }
	        
	        volunteer.getApplications().remove(applicationRepository.findByUserAndVolunteer(user, volunteer).get(0));
	        applicationRepository.deleteByUserAndVolunteer(user, volunteer);
	        updateCurrentUser(volunteer);
	        return false;
    }

VolunteerService 중 가장 많은 의존성을 가진 함수를 리팩토링 해보며 Facade를 이용한 코드는 어떤지 보도록 하자.

위 코드는 VolunteerRepository와 ApplicationRepository, UserRepository까지 3개의 레포지토리나 의존성을 갖는다. VolunteerService에는 VolunteerRepository만 있게 하고, Facade 패턴을 적용하여 이 함수의 기능을 수정하다보면 자연스레 위에서 의도한대로 리팩토링이 진행될 것이다.

우선 위 코드가 가진 문제점은 다음과 같다.

  • 이름이 isApplyVolunteer()면 단순하게 신청된 봉사인지만을 판별해야하는데, 다른 일까지 하고 true와 false를 반환한다.

이미 신청된 봉사는 취소하고, 신청하지 않은 상태에서는 신청하기 위해 이런식으로 코드를 짰던 것 같은데, 지금보니까 바람직하지 않다.

그럼 먼저 오류가 나도록 VolunteerFacade를 만들어 위 함수를 구현하고 이 오류들을 하나씩 해결해보는 방식으로 리팩토링을 진행해보자.

VolunteerFacade.applyVolunteer()

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35

@Slf4j
@RequiredArgsConstructor
@Service
public class VolunteerFacade {
	private final VolunteerService volunteerService;
	private final FavoriteService favoriteService;
	private final CommentService commentService;
	private final ApplicationService applicationService;
	private final UserService userService;
    
	@Transactional
	public Application applyVolunteer(Long volunteerId, UserEmailRequestDto emailRequestDto) {
		    User user = userService.getUserByEmail(emailRequestDto.getEmail());
		    Volunteer volunteer = volunteerService.getVolunteerById(volunteerId);
		
		    List<User> users = volunteer.getApplications().stream()
		            .map(Application::toDto)
		            .map(ApplyResponseDto::getUser)
		            .collect(Collectors.toList());
		
		    Application application = Application.builder()
		            .volunteer(volunteer)
		            .user(user)
		            .build();
		
		    if (!users.contains(user)) {
		        volunteer.addApplication(application);
		        return applicationService.applyVolunteer(application);
		    }
		
		    volunteer.removeApplication(application);
		    return applicationService.deleteByUserAndVolunteer(user, volunteer);
	}
}

위처럼 코드를 짜보았다. 이름도 applyVolunteeer기 때문에 직관적으로 봉사를 신청하는 메서드임을 알 수 있으며, Facade 클래스에서 Service를 조합하기 때문에 Service가 여러 레포지토리의 의존성을 가질 필요도 없어졌다!

물론 아직 ApplicationService와 VolunteerService, UserService가 없어서 오류 투성이지만, 하나씩 생성해보며 이 함수가 동작하도록 해보자.

VolunteerService.getVolunteerById()

1
2
3
4
5
6
7
8
9
10
11

@Slf4j
@RequiredArgsConstructor
@Service
public class VolunteerService {
	private final VolunteerRepositoy volunteerRepository;
	
	@Transactional(readOnly = true)
	public Volunteer getVolunteerById (Long id) {
	    	return volunteerRepository.findById(id).orElseThrow(NoExistUserException::new);
	}
}

UserService.getUserByEmail()

1
2
3
4
5
6
7
8
9
10
11
12
13

@Slf4j
@RequiredArgsConstructor
@Service
public class UserService {
	    private final static String NAVER_LOGIN_URL = "https://openapi.naver.com/v1/nid/me";
	    private final static String NAVER_LOGIN_HEADER_STRING = "Bearer ";
	
	    private final UserRepository userRepository;
	
	    public User getUserByEmail(String email) {
	        return userRepository.findByEmail(email).orElseThrow(NoExistUserException::new);
	    }
}

ApplicationService.applyVolunteer()

1
2
3
4
5
6
7
8
9

@RequiredArgsConstructor
@Service
public class ApplicationService {
	    private final ApplicationRepository applicationRepository;
	    
	    public Application applyVolunteer(Application application) {
	        return applicationRepository.save(application);
	    }
}

이렇게 각자의 레포지토리에만 의존성을 갖게 해주었다.

이렇게하면 클라이언트는 내부로직을 알 필요없이 VolunteerFacade에 있는 함수만 사용하면 되니까 좋고, 심지어 서비스마다 각자의 레포지토리에만 의존하므로 의존성을 덜어주었다. 새로운 기능이 필요하다면 Service 내부의 코드를 수정하는게 아니라 새로운 Facade객체를 만들거나, VolunteerFacade에서 Service들을 조합하여 원하는 결과를 가져다 주게 하면 된다.

변경한 커밋 링크 - [[링크] refactor - Facade 패턴을 적용하였습니다.] (https://github.com/Jamong-Project/Jamong-Backend/commit/c33b783344a18b4145eee774ad3f7b2b358c2a65)

마무리

이렇게 Facade 패턴을 적용해보며 리팩토링을 진행해보았다. 확실히 facade 패턴을 적용해보니 의존성이 덜어지고, 클래스가 훨씬 더 나누어 져서 깔끔해진 것 같다. 하지만 확실히 Service와 Repository의 의존성을 줄이고 VolunteerService 하나가 너무 많은 역활을 갖고 있던걸 여러 Service로 분리 할 수 있는 건 좋았지만, 결국 Facade 클래스의 책임이 너무 무거워지는 건 아닌지 고민하게 되었다. 이를 개선할 방법을 더욱 찾아봐야겠다는 생각이 들었다.

책임분리와 의존성 줄이기에 대해서만 고민해본 적은 없었던 것 같은데, 이번 기회에 SOLID 원칙을 실제로 적용하는 것이 얼마나 중요한지 깨달았고, 이를 통해 코딩을 함에있어서 기능 구현보다도 아키텍쳐가 중요하다는 것을 깨달았다. 기능을 만드는 것은 쉽지만, 그것을 오랫동안 유지보수하기 쉽고, 최대한 내부 코드 수정 없이 상속과 다형성을 이용해서 유연한 코드를 만드는 구조를 고민하는 것이 훨씬 어렵다. 그것이 바로 시니어 개발자가 돈을 많이 받는 이유일테고…

앞으로도 아키텍쳐에 대한 고민과 공부를 열심히 해야겠다는 생각이 들었다.

카테고리:

업데이트:

댓글남기기

참고

[BOJ] 2623 음악 프로그램 (Python3)

2 분 소요

문제 인터넷 방송 KOI(Korea Open Internet)의 음악 프로그램 PD인 남일이는 자기가 맡은 프로그램 ‘뮤직 KOI’에서 가수의 출연 순서를 정하는 일을 매우 골치 아파한다. 순서를 정하기 위해서는 많은 조건을 따져야 한다.

[BOJ] 1987 알파벳 (Python3)

1 분 소요

문제 세로 R칸, 가로 C칸으로 된 표 모양의 보드가 있다. 보드의 각 칸에는 대문자 알파벳이 하나씩 적혀 있고, 좌측 상단 칸 (1행 1열) 에는 말이 놓여 있다.