null을 반환하는 코드는 일거리를 늘릴 뿐만 아니라 호출자에게 문제를 떠넘긴다.

최근 클린 코드를 학습하고 있습니다.

 

고맙게도 회사에서 교육을 시켜주고 있어, 강한 의지 없이도 학습을 이어나갈 수 있게 해주네요.

 

클린 코드의 오류 처리 챕터에서,

 

null을 반환하는 코드는 일거리를 늘릴 뿐만 아니라 호출자에게 문제를 떠넘긴다.

 

라는 항목을 보고, 제가 그동안 고민해 왔던 것을 작성해보려고 합니다.

 

교재는 엉클밥의 클린 코드입니다. (물론 책으로 공부하진 않습니다..)

 

Java에서는 Optional이라는 Null을 예방하는, Java을 Null-safe 하게 만들어 주는 훌륭한 클래스가 있습니다.

 

저는 주로 JPARepository에서 Entity를 불러올 때 많이 사용하는데요,

 

제가 기존에 사용하는 방식의 예제를 보면 다음과 같습니다. (Null-Safe하지 않은 방식)

 

Stock이라는 Entity 클래스를 하나 만들고,

DTO인 StockRepository,

비지니스 로직을 담당할 StockService

사용자와 커뮤니케이션을 할 StockController를 만들었습니다. 

 

 

기존에 사용하는 방식

 

 

// Stock.java

import lombok.AccessLevel;
import lombok.Data;
import lombok.experimental.FieldDefaults;

import javax.persistence.Entity;
import javax.persistence.GeneratedValue;
import javax.persistence.GenerationType;
import javax.persistence.Id;

@Data
@FieldDefaults(level = AccessLevel.PRIVATE)
@Entity
public class Stock {

    @Id
    @GeneratedValue(strategy = GenerationType.IDENTITY)
    Integer id;

    String name;

    String price;
}

// StockRepository.java

import org.springframework.data.jpa.repository.JpaRepository;

public interface StockRepository extends JpaRepository<Stock, Integer> {
}

// StockService.java

public interface StockService {

    Stock getStock(Integer id);
}

// StockServiceImpl.java

import lombok.AccessLevel;
import lombok.RequiredArgsConstructor;
import lombok.experimental.FieldDefaults;
import org.springframework.stereotype.Service;

@Service
@FieldDefaults(level = AccessLevel.PRIVATE)
@RequiredArgsConstructor
public class StockServiceImpl implements StockService {

    final StockRepository stockRepository;

    @Override
    public Stock getStock(Integer id) {
        return stockRepository.findById(id).orElse(null);
    }
}

// StockController.java

import lombok.AccessLevel;
import lombok.RequiredArgsConstructor;
import lombok.experimental.FieldDefaults;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.RestController;

@RestController
@FieldDefaults(level = AccessLevel.PRIVATE)
@RequiredArgsConstructor
public class StockController {

    final StockService stockService;

    @GetMapping("/stocks/{id}")
    public ResponseEntity<Stock> getStock(@PathVariable("id") String id) {

        Stock stock = stockService.getStock(Integer.valueOf(id));

        return ResponseEntity.ok(stock);
    }
}

 

 

이번 블로그를 포스팅 하기 위해, Stock 이라는 항목을 새로 생성하고, DB에 데이터를 넣은 후 Client에서 호출하면 위 그림과 같은 결과가 나옵니다.

 

여기서 제가 계속 고민하던 부분이, 

 

// StockServiceImpl.java

    ...
    @Override
    public Stock getStock(Integer id) {
        return stockRepository.findById(id).orElse(null);
    }
    ...

이 부분이며, id로 찾은 후에 없으면 null을 반환하는 방식이죠.

 

저는 이 부분을 사용할 때, 별로 이상하다고 생각하지 않고 사용했었는데, 이번 교육을 들으면서 제가 null을 넘겨주는 것이 조금 무책임한게 아닌가 하는 생각이 들었습니다.

 

이제 getStock()을 사용하는 Controller에서 null을 유발시켜 봅시다.

 

// StockController.java

public class StockController {

    final StockService stockService;

    @GetMapping("/stocks/{id}")
    public ResponseEntity<Stock> getStock(@PathVariable("id") String id) {

        Stock stock = stockService.getStock(Integer.valueOf(id));

        String stockName = stock.getName();  // 이 부분이 추가됨 (null 유발)

        return ResponseEntity.ok(stock);
    }
}

후에 

http://localhost/stocks/2 를 실행시켜보면,  당연하게도 NPE가 발생합니다.

2022-07-26 22:33:44.622 ERROR 21956 --- [p-nio-80-exec-2] o.a.c.c.C.[.[.[/].[dispatcherServlet]    : Servlet.service() for servlet [dispatcherServlet] in context with path [] threw exception [Request processing failed; nested exception is java.lang.NullPointerException] with root cause

java.lang.NullPointerException: null
	at com.hughstudio.files.stock.StockController.getStock(StockController.java:23) ~[main/:na]
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[na:na]
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[na:na]
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[na:na]
	at java.base/java.lang.reflect.Method.invoke(Method.java:566) ~[na:na]
	at org.springframework.web.method.support.InvocableHandlerMethod.doInvoke(InvocableHandlerMethod.java:205) ~[spring-web-5.3.20.jar:5.3.20]
...

 

똑똑하다고 생각되는 IntelliJ IDEA Ultimate 에서도 warning을 표시해주지 않습니다.

사용자가 null check를 누락시킬 가능성이 높다고 생각됩니다.

 

 

NPE를 예방하기 위해 아래처럼 null check 로직을 넣으면 오류가 발생하지 않습니다.

// StockController.java

public class StockController {

    final StockService stockService;

    @GetMapping("/stocks/{id}")
    public ResponseEntity<Stock> getStock(@PathVariable("id") String id) {

        Stock stock = stockService.getStock(Integer.valueOf(id));

        // NPE 방지를 위한 null check
        if (Objects.nonNull(stock)) {
            String stockName = stock.getName();
        }

        return ResponseEntity.ok(stock);
    }
}

 

여기까지가 제가 기존에 사용하던 방식이고,

클린 코드에서는 의미를 담은 Exception을 발생시켜 사용자가 catch에서 예외처리를 하도록 하라고 되어 있습니다.

 

그럼 한 번 수정해보겠습니다.

 

// StockServiceImpl.java

public class StockServiceImpl implements StockService {

    final StockRepository stockRepository;

    @Override
    public Stock getStock(Integer id) throws Exception {        
        return stockRepository.findById(id).orElseThrow(() -> new Exception("Not found id " + id));
    }
}

우선은 Exception()을 생성하여, Not found id {id}를 전달하도록 하니, Controller쪽에서 바로 에러가 발생합니다.

 

getStock()에서 Exception을 throw 하고 있으니, try catch를 사용하라고 하네요.

무시하고 빌드하면 빌드가 되지 않아요.

 

이렇게 null 대신 exception을 발생시키면 null에는 더 안전할 수도 있겠습니다.

// StockService.java

public interface StockService {

    @NotNull
    Stock getStock(Integer id) throws Exception;
}

StockService에 @NotNull을 붙여주면 IDE에서도 확인이 가능합니다. 아 요녀석은 null을 return해주지 않는구나 라구요.

 

 

(추가)

사용자가 알아서 사용하도록 Optional<T>을 리턴해주는 것도 좋은 방안이라는 생각이 듭니다.

 

// StockServiceImpl.java

public class StockServiceImpl implements StockService {

    final StockRepository stockRepository;

    @Override
    public Optional<Stock> getStock(Integer id) {
        return stockRepository.findById(id);
    }
}

// StockController.java

public class StockController {

    final StockService stockService;

    @GetMapping("/stocks/{id}")
    public ResponseEntity<Stock> getStock(@PathVariable("id") String id) {

        Stock stock = stockService.getStock(Integer.valueOf(id)).orElse(null);

        if (Objects.nonNull(stock)) {
            String stockName = stock.getName();
        }

        return ResponseEntity.ok(stock);
    }
}