[클린 코드] 7장 - 오류 처리 (Error Handling)

이번 포스팅에선 클린 코드 7장 - 오류 처리를 정리해보겠습니다.

 

로직도 중요하지만 오류 처리를 잘 하는 것도 매우 중요합니다.

 

어떻게 오류 처리를 해야 좋을지 알아보겠습니다.

 

오류 코드보다 예외를 사용하라!

 

3장 8번에서 한번 다룬 내용이 7장에 또 나오네요.

 

아래 코드를 보면 getHandle(DEV1), record.getStatus()를 비교하는 로직을 보면 else일 때 에러 메세지를 로깅하고 있습니다.

 

문제점

 

호출 즉시 에러 처리를 하지않기 때문에 코드가 복잡해져 이해하기가 어렵다.

 

public class DeviceController {
    public void sendShutDown() {
        DeviceHandle handle = getHandle(DEV1);
        // 디바이스 상태를 점검한다.
        if (handle != DeviceHandle.INVALID) {
            // 레코드 필드에 디바이스 상태를 저장한다.
            retrieveDeviceRecord(handle);
            //디바이스가 일시정지 상태가 아니라면 종료한다.
            if (record.getStatus() != DEVICE_SUSPENDED) {
                pauseDevice(handle);
                clearDeviceWorkQueue(handle);
                closeDevice(handel);
            } else {
                logger.log("Device suspended. Unable to shut down");
            }
        } else {
            logger.log("Invalid handle for: " + DEV1.toString());
        }
    }
}

 

위 코드를 tryToShutDown 메소드로 추출해놓고 그걸 try/catch로 잡아 예외를 처리했다.

(tryToShutDown으로 추출한 이유는 오류도 한 가지 작업이기 때문이다.)

 

else에 예외 처리를 하는 것보다 throw new DeviceShutDownError 예외를 던져 처리하는 로직이 훨씬 깔끔하다.

 

로직을 보기보단 그냥 저런 코드보단 예외로 빼는 게 깔끔하다 정도로 이해하자!

 

public class DeviceController {
    public void sendShutDown() {
        try {
            tryToShutDown();
        } catch (DeviceShutDownError e) {
            logger.log(e);
        }
    }

    private void tryToShutDown() throws DeviceShutDownError {
        DeviceHandle handle = getHandle(DEV1);
        DeviceRecord record = retrieveDeviceRecord(handle);

        pauseDevice(handle);
        clearDeviceWorkQueue(handle);
        closeDevice(handle);
    }

    private DeviceHandle getHandle(DeviceId id) {
        ...    	
        throw new DeviceShutDownError("Invalid handle for:" + id.toString());
        ...
    }
}

 

Try-Catch-Finally 문부터 작성하라!

 

예외가 발생할 코드를 짜는 경우엔 try/catch문으로 시작하는게 좋다.

 

예)

 

파일이 없는 경우 예외를 던지는 단위 테스트에 맞춰 코드를 구현해보겠습니다. (TDD로 개발)

 

@Test(expected = StorageException.class)
public void retrieveSectionShouldThrowOnInvalidFileName() {
    sectionStore.retrieveSection("invalid - file");
}

 

아래 코드는 위 테스트가 성공하지 못 합니다. StorageException을 던지는 로직이 없습니다.

 

public List<RecordedGrip> retrieveSection(String sectionName) {
    // 실제로 구현할 때까지 비어 있는 더미를 반환
    return new ArrayList<RecordedGrip>();
}

 

파일이 없으면 Exception이 발생함으로 아래 로직은 테스트가 성공합니다.

 

public List<RecordedGrip> retrieveSection(String sectionName) {
    try {
        FileInputStream stream = new FileInputStream(sectionName);
    } catch (Exception e) {
        throw new StorageException("retrieval error", e);
    }
    return new ArrayList<RecordedGrip>();
}

 

위 코드를 리팩토링해보겠습니다.

 

1) FileInputStream stream = new FileInputStream(sectionName); 와 stream.close(); 사이에 stream을 읽어와서 처리하는 로직들을 넣을 수 있는 범위를 만들어 준다. 저 사이에 추가된 로직들은 예외가 발생하지 않는다고 생각한다.

 

2) 파일이 없으면 FileNotFoundException이 발생하기 때문에 Exception 범위를 줄여준다.

 

public List<RecordedGrip> retrieveSection(String sectionName) {
    try {
        FileInputStream stream = new FileInputStream(sectionName); // 1)
        stream.close();
    } catch (FileNotFoundException e) { // 2)
        throw new StorageException("retrieval error", e);
    }
    return new ArrayList<RecordedGrip>();
}

 

위처럼 평소에 TDD로 개발을 하지는 않지만 TDD로 안 해도 try/catch문으로 시작하는 것도 괜찮아보이네요. 처음부터 예외를 생각하고 짜는 습관을 길러야겠습니다.

 

unchecked 예외를 사용하라!

 

자바에 예외에는 checked, unchecked 2가지 예외가 존재하는데 checked 예외는 특별한 경우가 아니면 사용하지 말자.

 

 

checked 예외란?

 

checked 예외는 반드시 예외 처리를 해줘야 한다.

 

throws "checked 예외"가 명시되어있는 메소드를 호출하는 경우

 

 

아래 처럼 exception을 반드시 잡으라는 에러가 나와 반드시 처리를 해줘야한다.

 

 

checked 예외를 사용하지 말아야 하는 이유?

 

아래처럼 a -> b, b -> c를 부르는데 c가 checked 예외를 던지면 a, b 전부 예외를 잡기 위해 throws를 붙혀줘야 한다.

 

public void a() throws FileNotFoundException { // 또 예외를 잡고
    b();
}

private void b() throws FileNotFoundException { // 또 예외를 잡고
    c();
}

private void c() throws FileNotFoundException {
    FileInputStream stream = new FileInputStream("fileName");
}

 

처음부터 붙혔으면 그나마 괜찮은데 추후에 소스 수정이 일어나서 c에 다른 checked 예외를 추가한다면? a, b에 또 예외 처리를 추가해야 한다. 이는 OCP를 위반한다.

 

checked 대신 unchecked 예외를 사용하자!

 

예외에 의미를 제공하라!

 

소스 전체에서 이상한 인자를 받는 예외 로직에 IllegalArgumentException만 발생시킨다면?

 

도대체 어디서 에러가 발생했는지 stacktrace를 봐야만 알 수 있다.

 

추가로, IllegalArgumentException과 같이 자바에서 제공해주는 예외를 사용하면 다른 로직에서 발생했는지 내 로직에서 발생한 건지 알 수가 없다.

 

...
throw new IllegalArgumentException();
...

 

InvalidSearchArgumentException 같이 식별 가능한 예외를 정의하고 그 안에 에러 메세지를 넘겨주는 방식으로 변경하면 어디서 발생한 에러인지 짐작이라도 할 수가 있다.

 

 

...
throw new InvalidSearchArgumentException("검색조건은 빈칸일 수 없습니다.");
...

 

호출자를 고려해 예외 클래스를 정의하라!

 

ACMEPort.open() 메서드는 3개의 에러를 발생시키고 있다.

 

그래서 에러 처리를 하기 위해 아래와 같이 try/catch를 작성해야 한다.

 

ACMEPort port = new ACMEPort(12);

try {
    port.open();
} catch (DeviceResponseException e) {
    reportPortError(e);
    logger.log("Device response exception", e);
} catch (ATM1212UnlockedException e) {
    reportPortError(e);
    logger.log("Unlock response exception", e);
} catch (GMXError e) {
    reportPortError(e);
    logger.log("Device response exception", e);
} finally {
    ...
}

 

ACMEPort.open 메서드 사용하기가 너무 불편하다.

 

Wrapper 클래스를 이용해 이를 리팩토링해보겠다.

 

에러 처리 로직을 보면 reportPortError(e), logger.log("에러 메세지")를 하는 게 전부다.

 

reportPortError(e);
logger.log("Device response exception", e);

 

아래와 같이 3개의 예외를 PortDeviceFailure 예외로 통일시켜 처리하는 LocalPort 클래스를 만들었다.

 

public class LocalPort {
    private ACMEPort innerPort;

    public LocalPort(ACMEPort innerPort) {
        this.innerPort = innerPort;
    }

    public void open() {
        try {
            port.open();
        } catch (DeviceResponseException e) {
            throw new PortDeviceFailure(e);
        } catch (ATM1212UnlockedException e) {
            throw new PortDeviceFailure(e);
        } catch (GMXError e) {
            throw new PortDeviceFailure(e);
        }

        ...
    }
}

 

 

 

맨 처음 에러 3가지를 다 처리하려고 했던 코드보다 훨씬 깔끔해졌다.

 

LocalPort port = new LocalPort(12);

try {
    port.open();
} catch (PortDeviceFailure e) {
    reportError(e);
    logger.log(e.getMessage(), e);
} finally {
    ...
}

 

예외 처리 로직이 많은 경우 호출자를 고려해 wrapper 클래스로 깔끔하게 사용하자!

 

흐름을 끊지 말자!

 

아래 로직을 읽어보자.

 

id로 user를 가지고 와서 level 값을 넣어준다.

 

userNotFoundException을 잡아서 UserLevel.BASIC을 넣어준다. (?)

 

userLevel을 리턴한다.

 

위 로직은 user를 id로 찾아와서 있으면 getLevel을 없으면 BASIC을 리턴해라 라는 로직인데 try/catch로 되어있어서 코드를 이해하는 흐름을 방해한다. 

 

public UserLevel getUserLevel(Long id) {
    try {
        User user = userRepository.findById(id);
        return user.getLevel();
    } catch (UserNotFoundException e) {
        return UserLevel.BASIC;
    }
}

 

아래처럼 예외를 없애고 흐름에 따라 읽히게 사용하자. 이해하기가 더 쉬워졌다.

 

public UserLevel getUserLevelOrDefault(Long id) {
    User user = userRepository.findById(id);
    
    if (user == null) {
        return UserLevel.BASIC;
    } else {
        return user.getLevel();
    }
}

 

null을 반환하지 마라!

 

직원 리스트를 가져와 총 급여를 구하는 로직이다.

 

List<Employee> employees = getEmployees();

if (employee != null) {
    for(Employee e : employees) {
        totalPay += e.getPay();
    }
}

 

위 코드에서 getEmployees가 null을 반환해 if 절이 생겼는데 굳이 null을 반환할 필요가 있을까?

 

null인 경우 Collections.emptyList()를 리턴해주게 변경했다.

 

List<Employee> employees = getEmployees();

for(Employee e : employees) {
    totalPay += e.getPay();
}

 

너무 깔끔해졌고 NullPointeException이 발생할 가능성이 줄어들었다.

 

null을 전달하지 마라!

 

두 지점 사이의 거리를 구하는 메서드다.

 

public double xProjection(Point p1, Point p2) {
    return (p2.x - p1.x) * 1.5
}

 

위 메서드를 calculator.xProjection(null, new Point(12, 13)); 로 호출하면 어떻게 될까? 당연히 NPE가 발생한다.

 

public double xProjection(Point p1, Point p2) {
    if (p1 == null || p2 == null) {
        throw InvalidArgumentException("~~");
    }
    return (p2.x - p1.x) * 1.5
}

 

위처럼 null을 체크하면 그래도 예외처리가 된다. 이쁘진 않지만 필요하다.

 

또 다른 해결 방법으론 assert를 쓰는 방법이 있는데 별로라 따로 설명은 하지 않겠다.

 

위처럼 null을 넘기면 에러가 발생할 확률이 많기 때문에 넘기지 않는 방법이 가장 좋다.

 

 

예외 처리를 잘 하는 방법에 대해 알아봤습니다.

 

나름 괜찮은 방법들도 있고 좀 구시대적인 방법도 있는데 참고해서 잘 적용해야겠습니다.