这是一个多余的空指针异常捕获吗?

huangapple go评论75阅读模式
英文:

Is this a redundant NullPointerException catch?

问题

以下 Spring REST 代码根据给定的 ticketId 返回 List<TicketResponse>

在这段代码中,是否可能会抛出 NullPointerException

TicketController 中明确捕获了 NullPointerException

catch (NullPointerException nullPointerException) {
    throw new ResponseStatusException(
        HttpStatus.BAD_REQUEST, nullPointerException.getMessage(), nullPointerException);
}

思路可能是在检查票据 ID 时,如果为 null,则期望会抛出 NullPointerException,但实际上抛出的是 ResponseStatusException

如果变量 ticketId 是路径参数,由于未带有 ticketId 的基本 URL /,它永远不会为 null,我会收到:

There was an unexpected error (type=Method Not Allowed, status=405).

完整源码:

@RestController
public class TicketController {

    private final TicketServiceImpl ticketServiceImpl;

    public TicketController(TicketServiceImpl ticketServiceImpl) {
        this.ticketServiceImpl = ticketServiceImpl;
    }

    @GetMapping(path = "/{ticketId}")
    public ResponseEntity<List<TicketResponse>> getTicketsById(
        @PathVariable("ticketId") final Long ticketId) {
        try {
            final List<TicketResponse> ticketsById = ticketServiceImpl.getAll(ticketId);
            return new ResponseEntity<>(ticketsById, HttpStatus.OK);
        }
        catch (NullPointerException nullPointerException) {
            throw new ResponseStatusException(
                HttpStatus.BAD_REQUEST, nullPointerException.getMessage(), nullPointerException);
        }
        catch (TicketNotFoundException ticketNotFoundException) {
            throw new ResponseStatusException(
                HttpStatus.NOT_FOUND, "Ticket id not found",
                ticketNotFoundException);
        }
    }
}

@Service
public class TicketServiceImpl implements TicketService {

    private final TicketRepository ticketRepository;

    public TicketServiceImpl(TicketRepository ticketRepository) {
        this.ticketRepository = ticketRepository;
    }

    @Override
    public List<TicketResponse> getAll(Long ticketId) {

        final List<TicketResponse> ticketResponselist = ticketRepository.findData(ticketId);

        if (ticketId == null) {
            throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "ticket id cannot be null");
        }
        else if (ticketResponselist.size() == 0) {
            throw new TicketNotFoundException("ticket not found");
        }
        else {
            return ticketResponselist;
        }
    }
}

@Repository
public interface TicketRepository {

    public List<TicketResponse> findData(Long ticketId);

}
英文:

Below Spring REST code returns List<TicketResponse> for a given ticketId.

Could a NullPointerException be thrown in this code ?

NullPointerException explicitly caught in TicketController:

    catch (NullPointerException nullPointerException) {
throw new ResponseStatusException(
HttpStatus.BAD_REQUEST, nullPointerException.getMessage(), nullPointerException);
}

The thinking may have been when checking for null on the ticket id:

        if (ticketId == null) {
throw new ResponseStatusException(HttpStatus.BAD_REQUEST, &quot;ticket id cannot be null&quot;);
}

the expectation is that a NullPointerException would be thrown but instead a ResponseStatusException is thrown ?

If the variable ticketId is a path parameter it can never be null as hit the base url / without a ticketId I receive:

There was an unexpected error (type=Method Not Allowed, status=405).

Entire source:

@RestController
public class TicketController {
private final TicketServiceImpl ticketServiceImpl;
public TicketController(TicketServiceImpl ticketServiceImpl) {
this.ticketServiceImpl = ticketServiceImpl;
}
@GetMapping(path = &quot;/{ticketId}&quot;)
public ResponseEntity&lt;List&lt;TicketResponse&gt;&gt; getTicketsById(
@PathVariable(&quot;ticketId&quot;) final Long ticketId) {
try {
final List&lt;TicketResponse&gt; ticketsById = ticketServiceImpl.getAll(ticketId);
return new ResponseEntity&lt;&gt;(ticketsById, HttpStatus.OK);
}
catch (NullPointerException nullPointerException) {
throw new ResponseStatusException(
HttpStatus.BAD_REQUEST, nullPointerException.getMessage(), nullPointerException);
}
catch (TicketNotFoundException ticketNotFoundException) {
throw new ResponseStatusException(
HttpStatus.NOT_FOUND, &quot;Ticket id not found&quot;,
ticketNotFoundException);
}
}
}
@Service
public class TicketServiceImpl implements TicketService {
private final TicketRepository ticketRepository;
public TicketServiceImpl(TicketRepository ticketRepository) {
this.ticketRepository = ticketRepository;
}
@Override
public List&lt;TicketResponse&gt; getAll(Long ticketId) {
final List&lt;TicketResponse&gt; ticketResponselist = ticketRepository.findData(ticketId);
if (ticketId == null) {
throw new ResponseStatusException(HttpStatus.BAD_REQUEST, &quot;ticket id cannot be null&quot;);
}
else if (ticketResponselist.size() == 0) {
throw new TicketNotFoundException(&quot;ticket not found&quot;);
}
else {
return ticketResponselist;
}
}
}
@Repository
public interface TicketRepository {
public List&lt;TicketResponse&gt; findData(Long ticketId);
}

答案1

得分: 4

`if (ticketId == null)` 的检查应该发生在调用 `ticketRepository.findData(ticketId);` 之前。

否则,验证就没有意义。

另外,顺便提一下,捕获 `NullPointerException` 是一种不好的实践。原因是抛出空指针异常通常是代码存在问题的标志。代码应该是空指针安全的,可以使用 [Optional][1] 或者在方法级别进行适当的验证。在这种情况下,应该在路由级别进行验证(即外部输入)。从那时起,如果验证设置正确,你将处理非空的 id。

这也与从方法中返回 null 有关,这也是一种不好的实践,因为它要求在每个使用返回值的方法中进行检查。这会使代码变得混乱,引入新的抽象层次,并且通常会导致严重的错误。

  [1]: https://docs.oracle.com/javase/8/docs/api/java/util/Optional.html
英文:

The if (ticketId == null) check should happen before the ticketRepository.findData(ticketId); is called.

Otherwise, the validation doesn't make sense.

Also, as a side note, catching NullPointerException is a bad practice. The reason is that having a null pointer exception thrown is mostly a sign of a coding smell. The code should be null-safe, by either using e.g. Optional or having a proper validation at the method level. In this case, it would be at the route level (which is the external input). From that point onward, if the validation is set, you're dealing with non nullable id.

This is also somehow related to returning null from a method, which is also a bad practice, since it requires a check in every method which then uses the returned value. This would pollute the code, will introduce a new level of abstraction, and will generally lead to nasty bugs.

答案2

得分: 0

空指针异常检查在这种情况下不是必需的。

附注:在将其作为方法参数传递时,最好使用超类型,除非您非常确定不会抛出异常,而不是使用您自己的类型。

英文:

NullPointerException check is not required in this case.

Side note : Its better to use the super type when passing as a method parameter until and unless you are pretty sure that no exception will be thrown instead of yours.

huangapple
  • 本文由 发表于 2020年4月9日 01:52:15
  • 转载请务必保留本文链接:https://go.coder-hub.com/61106943.html
匿名

发表评论

匿名网友

:?: :razz: :sad: :evil: :!: :smile: :oops: :grin: :eek: :shock: :???: :cool: :lol: :mad: :twisted: :roll: :wink: :idea: :arrow: :neutral: :cry: :mrgreen:

确定