1

During the validation process, the following questions arose.

  1. Get one name with data.getName().

  2. And there is a procedure to verify that the value of this name is correct.

It's very simple.

But what if "data" itself is "null"?

It will try null.getName(), which will eventually result in NEC.

A question arose here.

First of all, NEC is wrong. = Not intentional.

However, when data is null, it can be interpreted that the value of name is incorrect (when it should not be null). = Serves purpose.

In other words, the fact that the logic is no longer executed in the section I want is the same.

However, an unintended NEC occurred, and I wonder if it can be interpreted as achieving my purpose.

How is it?

Is it correct to put additional logic to verify the data itself?

바보린
  • 159
  • 1
  • 13
  • 1
    That depends - for long-living code (and a lot of code is used for a surprisingly long time) it is important that the next person (or your own self in five years) can see on a glance what the intent is. For this reason, there are methods like `Objects.requireNonNull()` that allow clearly expressing intent, while not changing behavior (still throws NPE). – Hulk Apr 12 '22 at 06:26
  • There are other libraries that offer similar validation functions, such as Apache Commons, but the ideas are similar (The types of exceptions may vary, sometimes it's an `IllegalArgumentException`). – Hulk Apr 12 '22 at 06:28
  • Besides the readability, the second consideration is efficiency - if your method will fail anyway, it should fail as soon as possible, without doing other stuff that will be ignored anyway or that in worst case might have unintended sid effects. – Hulk Apr 12 '22 at 06:31
  • In the given case, I would say that using the `Optional` API is a good fit if the exception should be prevented and instead the validation should fail: `return Optional.ofNullable(data).map(Data::getName).map(this::validateName).orElse(false);` – Turing85 Apr 12 '22 at 06:32
  • I'm with Hulk here. If `data == null` is unintended this might hint at another problem and thus should be handled and probably be reported separately. If your logic is ok with treating that as an invalid name and go on that's fine but I'd not just swallow the potential red flag that a `null` value could be. Note that things might be different for `getName()`, i.e. your requirements could actually expect some names to be missing in which case treating `null` as an invalid name might be ok (but again: if that's unintended I'd check for this). – Thomas Apr 12 '22 at 06:35
  • This topic has also been extensively covered on softwareengineering, see e.g. https://softwareengineering.stackexchange.com/questions/64926/should-a-method-validate-its-parameters – Hulk Apr 12 '22 at 07:04

0 Answers0