다른 뭔가를 하기 싫어서 책을 훑어보다가 시작
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();
}- compact constructor
- 왠지 좀 아쉽네요 ..
- https://www.baeldung.com/java-record-keyword
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 메서드 맞춰주려다 보니 하게 된 작업 이예요 ㅎㅎㅎ
- 본래 IssueSearchParameter 역할 IssueSearchArgument 로 변경하고, 기존 IssueSearchParameter 내 중 SearchKeyMap 변환한 멤버 변수들은 IssueSearchParameter(IssueSearchArgument's property) 별도의 클래스로 분리
- 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.
이제는 생각 않기 - 끝 -
