한번씩 경험해본 토글 함수가 있을 것이다.(참고로 토글이란 on/off 를 번갈아가는 상태를 가지는 것을 이야기한다.)
예를들면 윈도우가 사용가능(enable)과 사용불가능(disable) 두 가지 상태를 가질 수 있다.
이것을 EnableWindow(handle)과 DisableWindow(handle)로 만드는 대신
EnableWindow(handle, boolean)으로 해서 두 번째 인자가 true이면 enable로 만들고 false이면 disable로 만드는 식이다.
Win32의 EnableWindow 함수도 그렇게 설계가 되었다.
BOOL EnableWindow(
HWND hWnd,
BOOL bEnable
);
오늘 개발을 하다가 이런 유혹에 살짝 빠졌다가 다른 방법으로 처리를 해서 정리해둔다.
요구사항은 Redis에 어떤 객체 값을 (짧은 임시기간) 저장을 해야했다. 그런데 일부 값이 민감정보(예. 생년월일, 카드번호) 같은 것이라서 암호화를 해야 했다.
이것을 어떻게 처리할까 하다가 처음에는 겸직함수를 쓰기로 했다.
모델 내에서 clone을 수행하되 특정 필드만 암호화를 적용해서 반환을 해주는 메서드를 추가했다.
public class Payload {
private final String orderId;
private final String cardNumber;
private final int amount;
private Payload(Payload original, Cryptography cryptography, boolean isEncrypt) {
orderId = original.orderId;
cardNumber = isEncrypt ? cryptography.encrypt(original.cardNumber) : cryptography.decrypt(original.cardNumber);
amount = original.amount;
}
public Payload cloneWith(Cryptography cryptography, boolean isEncrypt) {
return new Payload(this, cryptography, isEncrypt);
}
}
참고로 암호화 해주는 Cryptography라는 인터페이스는 단순히 아래와 같다.
public interface Cryptography {
String encrypt(String plain);
String decrypt(String encrypted);
}Redis에 저장하는 곳에서는 대략 아래와 같은 모습이다.
@Component
@RequiredArgsConstructor
public class PayloadCache {
private final RedisHelper redisHelper;
private final Cryptography cryptography;
public void save(Payload payload) {
String key = generateKey(payload.getOrderId());
Payload encrypted = payload.cloneWith(cryptography, true);
redisHelper.setObject(key, encrypted, 1, TimeUnit.MINUTES);
}
public Payload load(String orderId) {
String key = generateKey(orderId);
Payload encrypted = redisHelper.getObject(key);
return encrypted.cloneWith(cryptography, false);
}
private String generateKey(String orderId) {
return KEY_PREFIX + orderId;
}
}냄새가 난다
그런데 cloneWith라는 것이 사용하는 곳에서 보면 두 번째 인자가 무엇을 뜻하는 것인지 의미가 불명확했다.
또한 Payload의 생성자에서 삼항연산자 부분이 보기가 싫었다. 뭔가 중복된 필드명이 길었고, 사실 저 필드 말고 더 있었다.
(여기의 코드는 예를 들기 위해 단순화를 한 것이다.)
리팩토링
다음과 같이 명확한 이름을 주는 것이 좋을 것 같았다.
@Component
@RequiredArgsConstructor
public class PayloadCache {
private final RedisHelper redisHelper;
private final Cryptography cryptography;
public void save(Payload payload) {
String key = generateKey(payload.getOrderId());
Payload encrypted = payload.encryptWith(cryptography); // 여기
redisHelper.setObject(key, encrypted, 1, TimeUnit.MINUTES);
}
public Payload load(String orderId) {
String key = generateKey(orderId);
Payload encrypted = redisHelper.getObject(key);
return encrypted.decryptWith(cryptography); // 여기
}
// 나머지는 동일
}그렇다면 Payload는 아래의 방법이 최선일까 생각해보았다.
public class Payload {
private final String orderId;
private final String cardNumber;
private final int amount;
private Payload(Payload original, Cryptography cryptography, boolean isEncrypt) {
orderId = original.orderId;
cardNumber = isEncrypt ? cryptography.encrypt(original.cardNumber) : cryptography.decrypt(original.cardNumber);
amount = original.amount;
}
public Payload encryptWith(Cryptography cryptography) {
return new Payload(this, cryptography, true);
}
public Payload decryptWith(Cryptography cryptography) {
return new Payload(this, cryptography, false);
}
}
결론은...
다행히 암호화랑 복호화가 데이터 타입이 같았다. 그래서 동일 시그너처를 가지고 있었다.
파라미터를 Function을 받는 함수형 인터페이스로 바꾸어서 적용을 하니 훨씬 깔끔한 코드가 되었다.
public class Payload {
private final String orderId;
private final String cardNumber;
private final int amount;
private Payload(Payload original, Function<String, String> transformer) {
orderId = original.orderId;
cardNumber = transformer.apply(original.cardNumber);
amount = original.amount;
}
public Payload encryptWith(Cryptography cryptography) {
return new Payload(this, cryptography::encrypt);
}
public Payload decryptWith(Cryptography cryptography) {
return new Payload(this, cryptography::decrypt);
}
}
Sonarqube
나중에 소나큐브로 코드 분석을 했는데 아래와 같이 UnaryOperator<String>으로 바꾸라는 20시간짜리 Code Smell을 알려주었다.
이 문제는 별도의 스레드로 분리를 하였다.
RangeIntSpliterator
RangeIntSpliterator 클래스에서는 boolean으로 처리를 했다. 생성자에서 closed가 true면 내부적으로 1, false면 0을 last의 값으로 사용한다.
package java.util.stream;
final class Streams {
// ..
static final class RangeIntSpliterator implements Spliterator.OfInt {
RangeIntSpliterator(int from, int upTo, boolean closed) {
this(from, upTo, closed ? 1 : 0);
}
private RangeIntSpliterator(int from, int upTo, int last) {
this.from = from;
this.upTo = upTo;
this.last = last;
}
@Override
public boolean tryAdvance(IntConsumer consumer) {
Objects.requireNonNull(consumer);
final int i = from;
if (i < upTo) {
from++;
consumer.accept(i);
return true;
}
else if (last > 0) {
last = 0;
consumer.accept(i);
return true;
}
return false;
}위의 RangeIntSpliterator을 사용하고 있는 IntStream에선느 아래와 같이 호출을 하고 있다.
public interface IntStream extends BaseStream<Integer, IntStream> {
// ..
public static IntStream range(int startInclusive, int endExclusive) {
if (startInclusive >= endExclusive) {
return empty();
} else {
return StreamSupport.intStream(
new Streams.RangeIntSpliterator(startInclusive, endExclusive, false), false);
}
}
public static IntStream rangeClosed(int startInclusive, int endInclusive) {
if (startInclusive > endInclusive) {
return empty();
} else {
return StreamSupport.intStream(
new Streams.RangeIntSpliterator(startInclusive, endInclusive, true), false);
}
}