Engineering
리팩토링 여정, 더 나은 코드로의 항해 part 1. 리팩토링 계기와 문제 진단
Cinamon여기어때
2024년 12월 31일
원문에서 보기 ↗안녕하세요 😊 숙박전시개발팀 시나몬입니다.
이번에는 여기어때의 전시 리팩토링이 어떤 배경과 이유로 시작되었는지, 그리고 이를 통해 어떻게 발전을 이루어 왔는지 이야기를 나눠보려 합니다. 이 글이 비슷한 고민이나 경험을 가진 분들께 작은 도움이 되길 기대합니다.
숙박전시개발팀의 전시파트는 무슨 일을 하고있나요?
저희는 여기어때의 전반적인 제휴점 데이터 노출을 담당하고 있습니다. 제휴점 정보가 있다면 전시파트도 항상 함께합니다.앱 전시는 사용자 편의성을 높이기 위해 A/B 테스트 를 통해 다양한 실험을 진행하고 있는데요. 오늘은 이러한 A/B 테스트가 활발히 진행되면서 발생한 전시파트의 구조적 문제점에 대해 이야기해보려고 합니다. 그전에 간단하게 A/B 테스트가 무엇인지 알아볼까요?
A/B 테스트
A/B 테스트란, 사용자를 무작위로 나눈 뒤 서로 다른 버전을 제공하고 결과를 비교하여 더 나은 선택지를 찾아내는 실험 방식입니다. 여기어때는 이러한 A/B 테스트를 통해 새로운 서비스를 도입하기 전에 사용자 경험이 실제로 향상되는지 철저히 검증하고 있습니다. 그렇다면 제가 진행했던 A/B 테스트 중 가장 성공적이었던 카테고리 홈 개편 실험을 통해 구체적으로 살펴보겠습니다.
여기어때 카테고리홈 개편 실험
여기어때의 카테고리 홈은 지역별로 제휴점을 간편하게 탐색할 수 있다는 점이 큰 장점입니다. 하지만 테스트 이전의 카테고리 홈에서는 API 응답이 모두 반환되기 전까지 사용자가 아무런 행동도 할 수 없는 불편함이 있었습니다. 저 역시 카테고리 홈을 이용하다가 이탈했던 경험이 여러 번 있었는데요 상당히 불편했던 기억이 납니다.
이러한 문제를 개선하고 사용자 경험을 향상하기 위해 여기어때는 여러 그룹으로 나누어 A/B 테스트를 진행했습니다. 그 결과 가장 긍정적인 반응을 얻은 D그룹을 최종적으로 선택해 카테고리 홈을 개선할 수 있었습니다.
전시파트는 무엇이 문제인가요?
앞서 소개드린 것처럼 전시파트는 다양한 A/B 테스트 실험을 진행하고 있습니다. 거의 모든 변경 사항이 A/B 테스트를 거친다고 해도 과언이 아닙니다. 하지만 A/B 테스트 이전에는 문제가 되지 않았던 상속 구조 와 히스토리 부재로 인해 전시파트는 어려움을 겪고 있었습니다.
1. 전시 프로젝트의 상속구조와 재사용의 문제점
응답까지의 흐름을 요약하면 위와 같이 간단해 보입니다.하지만 Mapper와 Service는 모두 만능 클래스 역할을 하는 부모 클래스 를 상속받고 있다는 문제가 있습니다. 게다가, 응답 구조조차 서로 다른 페이지가 같은 클래스를 공유하며 사용하고 있는 상황입니다.
1.1 Response 클래스의 과한 재사용
기존 여기어때의 제휴점 데이터는 구조가 서로 거의 동일하여 모두 동일한 응답 클래스를 사용하여도 문제가 없었습니다. 그러나 현재는 A/B 테스트가 활발하게 진행되면서 각 페이지의 응답 구조가 점점 달라지기 시작했습니다.
@Value
@Builder(toBuilder = true)
@JsonInclude(JsonInclude.Include.NON_NULL)
public class ResponseWrapper implements Serializable {
String title;
String badge;
Meta meta;
Typo typo;
Empty empty;
@Builder.Default
List<? extends AbstractPlaceListItemModel> items = Collections.emptyList();
@Builder.Default
List<Experiment> hackles = Collections.emptyList();
}
위 코드는 공통으로 사용되고 있는 응답 클래스의 예시입니다. 하지만, 특정 페이지에서만 사용하는 필드들이 점점 추가되면서 클래스의 용도가 모호해지는 문제가 발생했습니다. 각자 다른 페이지에서 사용하는 필드를 공용 클래스에 추가하고, null 여부를 통해 노출할 필드를 결정했기 때문입니다.
이런 구조는 필드의 히스토리가 단절되면서 해당 필드가 어떤 목적으로 사용되는지 파악하기 어려운 상황을 초래했습니다. 특히 특정 필드의 사용 목적을 확인하려면 null을 할당한 코드를 일일이 추적해야만 했으며, 이는 유지보수와 확장성을 크게 저해하는 결과를 낳았습니다.
1.2 거대한 부모 클래스가 가져오는 문제점 (1)
위 이미지처럼 모든 서비스 클래스들이 AbstractService를 상속받고 있습니다. 하지만 AbstractService 내부에는 복잡한 제휴점별 전략과 비즈니스 로직이 강하게 녹아들어 있어 수정이 어려운 구조를 띠고 있었습니다.이로 인해 정책을 충분히 이해하지 못한 상태에서 코드를 작성하는 경우도 빈번히 발생했는데요.
겉으로 보기에는 간단해 보이는 이 구조는 실제로는 수많은 클래스들이 상속관계로 엮여 있어 전부 나열하기조차 어려웠습니다. 결국 유지보수와 확장이 어려워지는 문제가 드러나기 시작했습니다.
public abstract class AbstractService {
private final Map<Strategy, Function<...> strategies = Maps.newHashMap();
@PostConstruct
public void init() {
strategies.put(Strategy.ONLY_ADVERTISEMENTS, param -> (search, content) -> {});
strategies.put(Strategy.PLACE, param -> (search, content) -> {});
strategies.put(Strategy.NON_ADVERTISE, param -> (search, content) -> {});
strategies.put(Strategy.WITH_ADVERTISE, param -> (search, content) -> {});
}
}
예시 코드만 보면 간단해 보이지만, 각 전략에는 복잡한 비즈니스 로직 이 강하게 녹아 있어 A/B 테스트를 적용하기 어려운 구조였습니다. 새로운 기능을 추가하려면 기존 전략을 수정해야 했지만 이러한 전략은 AbstractService를 상속받는 모든 서비스 클래스 에서 공유되었기 때문에 변경의 여파가 너무 컸습니다.
또한, 데이터 노출 정책을 적용하기 위해서는 반드시 AbstractService를 상속해야만 했고 이로 인해 코드의 의존성이 점점 증가하며 구조가 악화되었습니다. 결과적으로 유연하게 확장하거나 테스트하기 어려운 단단하게 결합된 구조로 인해 유지보수의 부담이 가중되면서 저희는 점점 지쳐갔습니다..
1.3 거대한 부모 클래스가 가져오는 문제점 (2)
Mapper 역시 많은 클래스들이 만능 클래스 역할을 하는 부모 클래스인 PlaceMapper를 상속하고 있습니다. 데이터 변경을 위한 A/B 테스트를 진행하려면 결국 Mapper를 수정해야만 했습니다. 문제는 PlaceMapper를 상속받은 자식 클래스들이 타입 구분 외에는 별다른 역할이 없다는 점입니다.
구현 클래스는 사실상 아무런 기능도 하지 못하고, 모든 핵심 로직과 역할이 추상 클래스인 PlaceMapper에 집중되어 있었습니다. 이로 인해 부모 클래스의 강한 의존성 이 구조를 더욱 단단하게 만들었고 작은 변경도 전체 코드에 큰 영향을 미치게 되었습니다. 결국 테스트와 확장 모두에서 불편함이 드러나며 유연성과 유지보수성이 크게 저하되는 결과를 낳았습니다.
public abstract class PlaceMapper {
protected RoomMapper roomMapper;
protected PromotionMapper promotionMapper;
protected MetaMapper metaMapper;
protected BrazeMapper brazeMapper;
protected EmblemMapper emblemMapper;
protected FavoriteMapper favoriteMapper;
public PlaceMapper(..) {
}
public ListModel mapPlaceToList(..) {
// 필요한 속성을 수정한 Place 객체 생성
Place revisedPlace = modifyPlaceAttributes(..);
// 리스트 모델 생성 후 반환
return buildPLPModel(..);
}
// Place 객체의 속성을 수정하는 메서드
private Place modifyPlaceAttributes(..) {
// 입력값을 기반으로 Place 속성 조정
return place;
}
// 리스트 모델을 구성하는 메서드
private ListModel buildListModel(..) {
// 다양한 매퍼에서 데이터를 결합하여 모델 생성
return new PLPListModel();
}
// 카테고리를 반환하는 추상 메서드, 서브클래스에서 구현
public abstract Category getCategory();
}
//모텔 타입의 구현체
public class MotelMapper extends PlaceMapper {
public MotelMapper(..) {
super(..);
}
@Override
public ListModel of(..) {
throw new UnsupportedOperationException("Not implemented");
}
@Override
public Category category() {
return Category.MOTEL;
}
}
//호텔 타입의 구현체
@Component
public class HotelMapper extends PlaceMapper {
public HotelMapper(..) {
super(..);
}
@Override
public Category category() {
return Category.HOTEL;
}
}
//펜션 타입의 구현체
public class PensionMapper extends PlaceMapper {
public PensionMapper(..) {
super(..);
}
@Override
public Category category() {
return Category.PENSION;
}
}
//캠핑 타입의 구현체
public class CampingMapper extends PlaceMapper {
public CampingMapper(..) {
super(..);
}
@Override
public Category category() {
return Category.CAMPING;
}
}
위 코드에서도 볼 수 있듯이, 구현 클래스들은 최소한의 역할만 수행하고 있고, 실질적인 핵심 비즈니스 로직은 모두 부모 클래스인 PlaceMapper에 집중되어 있습니다.이로 인해 자식 클래스들은 타입 구분 정도의 역할에 그치며 부모 클래스에 대한 의존도가 지나치게 높아진 상태입니다.
2. 리팩토링을 결심하게 된 이유가 무엇인가요?
기존의 확장하기 어려운 구조 는 여러 문제를 야기했습니다.히스토리가 단절된 실험 코드를 제거하는 데 불필요한 시간 이 소요되었고 예상치 못한 페이지에서 갑작스러운 이슈가 발생하는 일도 잦았습니다.
2.1 예측할 수 없는 변경여파
입사 후 두 번째로 진행했던 실험에서 잊지 못할 사건이 있었습니다. 작업한 실험을 운영 환경에 배포한 직후, 카테고리 홈의 기획전이 노출되지 않는다는 메시지를 받았습니다. 저는 당시 검색 페이지의 광고 구좌 작업만 맡았기 때문에 카테고리 홈과는 무관하다고 생각했습니다. 모든 클래스를 파라미터를 제외하고 분리했기 때문입니다.
확신의 말..
원인을 분석해보니 결국 제 작업이 문제였습니다. 광고 구좌 작업 중에isEmptyLocation이라는 메소드를 추가했는데, 배포 후 emptyLocation을 찾을 수 없다는 에러 메시지를 발견했습니다.
이 문제는 리플렉션 과정에서 발생했는데요. 리플렉션은 isEmptyLocation 메소드를 필드처럼 해석해 emptyLocation이라는 필드를 찾으려 했던 것입니다. 그런데 왜 리플랙션 문제가 발생했을까요? 전시파트의 코드구조에서는 발생하지 않았어야 할 문제였습니다.
// API 호출을 위한 base param class
class BaseParam {
private CheckInOut checkInOut;
private Location location;
private PageParam pageParam;
private Map<String, String> headers;
}
// 검색 API 호출을 위한 param class
class BaseListParam extends BaseParam {
private BaseCategory category;
private String filters;
private String likes;
private String reservationActive;
private SearchType searchType;
private SortType sortType;
// 신규 필드 추가
private Boolean isUserLocation; // 광고 구좌 실험용 신규 추가 필드 !!
// 광고 구좌 실험 신규 추가 메서드 !!
public boolean isEmptyLocation() {
return !isUserLocation;
}
// 기존 생성자
public BaseListParam(BaseCategory category, String filters, SortType sortType) {
this.category = category;
this.filters = filters;
this.likes = likes;
this.reservationActive = reservationActive;
this.searchType = searchType;
this.sortType = sortType;
}
// 신규 광고 구좌 실험 생성자
public BaseListParam(BaseCategory category, String filters, SortType sortType, Boolean isUserLocation) {
this.category = category;
this.filters = filters;
this.likes = likes;
this.reservationActive = reservationActive;
this.searchType = searchType;
this.sortType = sortType;
this.isUserLocation = isUserLocation;
}
}
// plp param 구현체 (변경 X)
class CategoryPlaceListParam extends BaseListParam {
public CategoryPlaceListParam(BaseCategory category, String filters, String likes, String bedTypes, String grades, String campings,
String themes, String reservationActive, SearchType searchType, SortType sortType) {
super(category, filters, likes, bedTypes, grades, campings, themes, reservationActive, searchType, sortType);
}
}
// plp param 신규 구현체
class CarouselPlaceListParam extends BaseListParam {
public CarouselPlaceListParam(BaseCategory category, String filters, String likes, String bedTypes, String grades, String campings,
String themes, String reservationActive, SearchType searchType, SortType sortType, boolean isUserLocation) {
super(category, filters, likes, bedTypes, grades, campings, themes, reservationActive, searchType, sortType, isUserLocation);
}
}
// search api parameter class 변환 mapper
@Mapper(componentModel = "spring")
interface SearchAPIMapper {
PlaceSearchListParam ofPlaceSearch(CategoryPlaceListParam param);
}
카테고리 기획전과 검색 페이지는 본질적으로 서로 다른 목적을 가진 기능입니다. 그러나 카테고리 기획전에서 검색 페이지의 파라미터를 재활용하려는 과정에서 리플렉션을 활용해 변환을 시도한 것이 문제의 핵심이었습니다.
isUserLocation은 생성자를 통해서만 null 체크가 이루어지는 구조였지만, 리플렉션을 사용하여 변환하면서 isUserLocation 값이 null로 설정되는 상황이 발생했습니다. 이로 인해 예상치 못한 NullPointerException(NPE)이 발생하게 된 것입니다.
문제를 단계적으로 살펴보면 다음과 같습니다.
- 리플렉션을 통해 변환하는 과정에서 존재하지 않는 isEmptyLocation 필드를 호출
- isEmptyLocation 내부에서 null 상태의 isUserLocation을 참조하면서 NPE 발생
이 문제는 단순히 리플렉션 사용의 한계 때문만이 아니라, 각 기능의 역할을 명확히 구분하지 않고 재사용에만 치중한 설계의 결과로 발생했습니다. 이는 서비스의 안정성을 심각하게 훼손한 사례 중 하나였습니다.
2.2 실험이 끝난 코드 정리만 한달
124번 실험이 종료되고 종료된 실험 결과를 코드에 녹여야하는 상황이 있었습니다. 그런데 담당자는 퇴사한 상태이고 히스토리는 남아있지 않았습니다. 124번 실험은 단순히 제휴점의 데이터를 변경하는 실험이었습니다. 그러나 제휴점 데이터는 여러 타입이 존재하고 노출하는 페이지마다 정책도 미세하게 다릅니다. 결국 저희는 아래와 같은 방법을 통해 이슈를 줄여 실험을 제거했습니다.
- 어떤 API에서 124번 실험이 담긴 코드를 호출하는지 로깅
- APP팀과 PO 분들에게 실험 코드 정리에 따른 협조 요청 및 QA 요청
- 주변 제휴점, 카테고리 제휴점 등등 신규 UX 적용건으로 필요한 코드 정리
복잡하게 얽혀있는 구조가 아니었다면 분명 쉽게 정리할 수 있었던 코드입니다. 그러나 저희는 A/B 테스트를 받아드릴 준비가 안 되어있었고 이를 개선하는 것이 숙박전시개발팀 전시파트의 미션이었습니다.
3. 이렇게 변할 거예요
현재 구조에서는 A/B 테스트를 원활히 진행하기 어려운 상황이라는 결론에 도달했습니다. 이에 따라 문제점을 면밀히 진단하고 필요한 작업들을 체계적으로 정리했습니다. 우리가 지향하는 방향은 ‘아름다운 코드’에만 집중하는 것이 아니라, A/B 테스트를 신속히 진행할 수 있는 구조를 구축해 여기어때 서비스의 요구에 유연하게 대응하는 것입니다. 레거시 코드는 수용하되 현재 서비스 방향에 부합하지 않는 구조는 과감히 정리하기로 결정했습니다.
▶︎ AS-IS
- 전시 파트 도메인에 대한 깊이 있는 이해를 가진 인력이 부족해 팀 협업에 어려움이 있음
- 응답, 파라미터, 서비스가 모두 만능 클래스를 상속하는 구조로 인해 확장이 어려움
- 통합 테스트 코드가 없어 새로운 기능 배포 시마다 불안감이 큼
- 각 모듈의 정책 문서가 파편화되어 효율적인 관리가 어려움
▶︎ TO-BE
- Mob Programming을 통해 리팩토링을 진행하며 팀원의 도메인 지식 공유 및 동기화
- 테스트 커버리지를 80% 이상 달성하고 E2E 테스트를 필수화
- A/B 테스트를 신속히 진행할 수 있도록 확장 가능한 구조로 개편
- 파라미터의 상속 관계를 제거하고 포함 관계로 개선
- 만능 클래스를 역할에 맞는 클래스로 분리하여 책임을 명확히 함
- 쉐도잉 작업을 통해 데이터 무결성과 검증을 강화
Part.1에선 전시파트가 어떤 문제점을 겪고 있었고, 어떻게 해결할 것인지 간단히 알아보았습니다. 리팩토링 결과는 티모의 “리팩토링 여정, 더 나은 코드로의 항해 part 2. 새로운 구조로 나아가는 길”에서 확인하실 수 있습니다.
감사합니다 😆