TIL 이팩티브 자바 읽다가 리팩토링

sally·2023년 8월 27일
0

JAVA

목록 보기
4/4

다른 뭔가를 하기 싫어서 책을 훑어보다가 시작

item 1~10 까지 보면서 관련되어 생각나는 코드들 수정하다가..
(이팩티브 책은 이런식으로 쭉~ 못 읽는)
item6 에서 그렇게 다른 길로 갔습니다. ㅎㅎㅎ

의식의 흐름대로 리팩토링 과정이니 ...
좋은 지식 알려주시면 감사하겠습니다. 😅

ITEM 6. 불필요한 객체 생성을 피하라

(사실 ITEM 6 가 주제는 아닙니다.... 🙄)
기존 문제의 코드 🌱

부담되나 봐요
요즘은 내부 좀 보고 써야 하는 클래스들이 보이네요.(어렵다)

( Pattern 만 기억 나고 … 다른 부분도 생각 났다면 좋았을 텐데(?)…. 🙈)

추상 클래스

인터페이스에 Pattern 초기화 시켜놓는 게 불편하고,

default 메서드 쓰면서 하나의 parsing을 추상 클래스 놓고 완성한 searchParameter 반환 위해 create 위임 하자니 억지로 마춘 듯 하고
그냥 추상클래스로 리팩토링 했습니다

public abstract class IssueSearch {
	public IssueSearchParameter from(IssueSearchRequest request) {
		final String text = request.getText();
		final int firstKeySeparatorIdx = text.indexOf('+');
		final IssueStatus status = toIssueStatus(text.substring(0, firstKeySeparatorIdx));
		final String behindSearchWords = text.substring(firstKeySeparatorIdx);
		final Map<SearchKeyType, String> parameters = this.parsing(behindSearchWords);
		return IssueSearchParameter.of(status, parameters);
	}

	protected abstract Map<SearchKeyType, String> parsing(String searchWords);
public class IssueSearchParsing extends IssueSearch {
	private static final Pattern PATTERN_OF_KEY = Pattern.compile("[+](.*?)[:]");
	private static final Pattern PATTERN_OF_VALUE = Pattern.compile("[:](.*?)[+]");

	@Override
	protected Map<SearchKeyType, String> parsing(String searchWords) {
		final Matcher matcherOfKey = PATTERN_OF_KEY.matcher(searchWords);
		final Matcher matcherOfValue = PATTERN_OF_VALUE.matcher(searchWords);
		SearchKeyMap searchKeyMap = SearchKeyMap.newInstance();

		while (matcherOfKey.find() && matcherOfValue.find()) {
			String key = matcherOfKey.group(1);
			String value = matcherOfValue.group(1);
			if (Strings.isBlank(key) || Strings.isBlank(value)) {
				break;
			}
			checkSearchKey(key, value);
			searchKeyMap.updateValue(SearchKeyType.getKey(key), trimComma(value));
		}

		final String lastKey = matcherOfKey.group(1);
		final String lastValue = searchWords.substring(searchWords.lastIndexOf(':') + 1);
		searchKeyMap.updateValue(SearchKeyType.getKey(lastKey), trimComma(lastValue));
		return searchKeyMap.parameters();
	}

generic

(아마 .. 여기서 끝냈을 텐데 .. 오늘 왠지 시간은 남고, 다른 건(?) 하기 싫고 …..)

요기조기 고치고 보니 크게 3가지가 변경 될 수 있겠다 싶어 졌어요

  • 요청한 검색키의 default 값
    • 서비스 별 검색 처리
    • 검색 조건
  • parsing 로직
  • 반환 파라미터들
    • IssueStatus 재활용 문제

반환 타입이 명확하지 않아서 방법 없나 고민하다 털고 일어나 세수하다가 스쳐지나갔어요. 다시 앉기 😑

public interface Searching<T> {
   default <T> T from(RequestedSearchTerms text) {
      return parseIntoParameters(text.getText());
   }

   <T> T parseIntoParameters(String text);
}

당시 기억이 새록새록 납니다.
갑자기 변경됐고, 시간은 여유치 않았던 ... 걍 util 클래스만 생각하면서 작업 했었는데, 검색 로직이 공통으로 쓰일 수 있지 않을까 싶어졌어요.

유동적으로 키와 값을 담을 SearchKeyMap 이 제 입장에서는 명확하지 않아 인터페이스는 부담됐고,
기존 SearchKeyType 을 SearchKey 인터페이스로 하고 각 검색어 별 enum 클래스 사용하게 해봤습니다. (뭐 알고 하는 거 아니라.. 의식의 흐름 ... )

// SearchKeyMap 
public Map<SearchKey, String> parameters() {
   //return new HashMap<>(searchWords);
   return searchWords.keySet().stream()
			.collect(toUnmodifiableMap(searchKey -> searchKey, searchWords::get));
}


// Searching<IssueSearchParameter>  toParameters() 리턴
IssueSearchParameter.of(status, this.parsing(behindSearchWords).parameters());
  • SearchKeyMap parameters() 는 조회 대상으로 불변타입으로 반환하고자 했는데, Guava 가 좋아보였습니다.
    (저는 단순 자바 프로그래밍 기준이라 추가하진 않았는데 .. 나중에 다른걸.. ㅎㅎㅎ)

record

기존 프로젝트와 자바 버전 다르게 해서 새로운 클래스도 써 봤습니다.
(😑 나는 왜 일을 만드는가)

당시 받은 검색값이 null 이 더라도 지정된 기본 검색어 반환 하게 했는데,
record 쓰면서 이러한 부분이 고민 됐습니다.

  • 인터페이스 주석 통해 getText() 주요 역할 알더라도 외부에서 record 제공 paramters() 사용 가능
public record IssueSearchRequest(String parameters) implements RequestedSearchTerms {
   @Override
   public String getText() {
      if (Strings.isBlank(parameters)) {
         return "is:open";
      }
      return parameters;
   }
}
  • parameters() 는 결국 default 값없이 반환 되버리는 결과 ..
@Test
void record_parameterIsNull_defaultParameter() {
	IssueSearchRequest issueSearchRequest = new IssueSearchRequest(null);
	String actual = issueSearchRequest.getText();

	assertThat(actual).isNotBlank();
}

@Test
void record_parameterIsNull_returnNull() {
   IssueSearchRequest issueSearchRequest = new IssueSearchRequest(null);
   String actual = issueSearchRequest.parameters();

	 assertThat(actual).isBlank();
}
public record IssueSearchRequest(String parameters) implements RequestedSearchTerms {
   public IssueSearchRequest {
      Objects.requireNonNull(parameters);
   }

   @Override
   public String getText() {
      if (Strings.isBlank(parameters)) {
         return "is:open";
      }
      return parameters;
   }
}

parse() 알고리즘 교체

parsing 부분만 생각하면 전략패턴이 적절해 보였습니다.

  • 반환타입 추상화가 아직 명확하지 않아 그냥 구현체 유지 - SearchKeyMap
public class IssueSearching implements Searching<IssueSearchParameter> {
	private final ParsingStrategy parsingStrategy;

	public IssueSearching(ParsingStrategy parsingStrategy) {
		this.parsingStrategy = parsingStrategy;
	}

	@Override
	public IssueSearchParameter parseIntoParameters(String text) {
		final int firstKeySeparatorIdx = text.indexOf('+');
		final IssueStatus status = toIssueStatus(text.substring(0, firstKeySeparatorIdx));
		final String behindSearchWords = text.substring(firstKeySeparatorIdx);
		return IssueSearchParameter.of(status, this.parsingStrategy.parse(behindSearchWords).parameters());

리팩토링 시작하게 된 Pattern.compile() 이나 세부적인 문자열 처리 로직들이 분리 되어져서 보다 간편해 졌어요
(처음에 한 번에 생각 나면 좋았을 텐데 ..)

Mapstruct

처음부터 parsingStrategy 가 반환하는 SearchKeyMap 이나, 제네릭을 쓰면서 반환 타입 고치고 싶은데 손이 안 대어 지고, 또 파라미터도 내가 원하는 enum 이나, 형식이 자유롭지 않을까 싶고, IssueSearchParameter 같이 일일이 객체 안에 넘어온 값별 매핑 처리가 불편하지 않을까 싶었어요.

(집으로 걸어오는 길에 생각나서 ... 아, 내일도 해봐야 겠네 😑 안 끝났어)

  • IssueSearchParameter → IssueSearchArgument: record 로 변경
    • 본래 IssueSearchParameter 역할 IssueSearchArgument 로 변경하고, 기존 IssueSearchParameter 내 중 SearchKeyMap 변환한 멤버 변수들은 IssueSearchParameter(IssueSearchArgument's property) 별도의 클래스로 분리
      (naming...sorry)
    • Mapstruct 에서 default 메서드 맞춰주려다 보니 하게 된 작업 이예요 ㅎㅎㅎ
  • SearchKeyMap.parameters() 호출 하든, SearchKeyMap 반환 하든 클라이언트에 노출 될 필요는 없지 않을까 싶었습니다.
public class IssueSearching implements Searching<IssueSearchArgument> {
	private final ParsingStrategy parsingStrategy;

	public IssueSearching(ParsingStrategy parsingStrategy) {
		this.parsingStrategy = parsingStrategy;
	}

	@Override
	public IssueSearchArgument parseIntoParameters(String text) {
		final int firstKeySeparatorIdx = text.indexOf('+');
		final IssueStatus status = toIssueStatus(text.substring(0, firstKeySeparatorIdx));
		final String behindSearchWords = text.substring(firstKeySeparatorIdx);
		return IssueSearchParamMapper.INSTANCE.toIssueSearchArgument(
			status,
			this.parsingStrategy.parse(behindSearchWords));
	}
  • IssueSearchParamMapper 를 SearchMapper 로 추상화 한다면,
    파라미터 변경 외에는 변경영향이 줄어들거 같았습니다.
@Mapper
public interface IssueSearchParamMapper {
	IssueSearchParamMapper INSTANCE = Mappers.getMapper( IssueSearchParamMapper.class );

	@Mapping(source = "issueStatus", target = "status")
	@Mapping(source = "map", target = "parameter")
	IssueSearchArgument toIssueSearchArgument(IssueStatus issueStatus, SearchKeyMap map);

	default IssueSearchParameter mapToObject(SearchKeyMap map) {
		final Map<SearchKey, String> parameters = map.parameters();
		return new IssueSearchParameter(
			parameters.get(IssueSearchKey.ASSIGNEE),
			parameters.get(IssueSearchKey.AUTHOR),
			parameters.get(IssueSearchKey.MILESTONE),
			parameters.get(IssueSearchKey.LABEL),
			parameters.get(IssueSearchKey.NONE)
		);
	}
}

(Mapstruct는 처음 써봐서 .. ㅎㅎ)
mapstruct.org
Example 8. Mapper which defines a custom mapping with a default method

@Mapper
public interface CarMapper {

    @Mapping(...)
    ...
    CarDto carToCarDto(Car car);

    default PersonDto personToPersonDto(Person person) {
        //hand-written mapping logic
    }
}

The class generated by MapStruct implements the method carToCarDto(). The generated code in carToCarDto() will invoke the manually implemented personToPersonDto() method when mapping the driver attribute.

이제는 생각 않기 - 끝 -

profile
sally의 법칙을 따르는 bug Duck

0개의 댓글