What are the disadvantages of protecting a string of objects from a nullPointerException using the optional map?

Asked

Viewed 145 times

-2

For example:

public static void main(String[] args) {
    Cidade cidade = new Cidade();
    Estado estado = new Estado(cidade);
    Endereco endereco = new Endereco(estado);
    Loja loja = new Loja(endereco);

    String nomeCidade;

    //null-safe ifs
    if(loja != null) {
        Endereco endereco1 = loja.getEndereco();

        if(endereco1 != null) {
            Estado estado1 = endereco1.getEstado();

            if(estado1 != null) {
                Cidade cidade1 = estado1.getCidade();

                if(cidade1 != null) {
                   nomeCidade = cidade1.getNome();
                }
            }
        }
    }

    //null-safe optional

    Optional<Loja> lojaOptional = Optional.of(loja);

    nomeCidade = lojaOptional.map(Loja::getEndereco).map(Endereco::getEstado).map(Estado::getCidade).map(Cidade::getNome).orElse("SEM CIDADE CADASTRADA");
}

The goal is to validate this object chain without needing a string of ifs.

  • Helloworld, it was even intended to unmark Maniero’s response as accepted, in favor of my?

2 answers

4

The biggest drawback I see is that it’s not very readable.

But legibility has to do with context. If everyone does it can become normal.

But I look at this and I don’t know what it does even though I have almost 40 years of experience and I know the functional language well. Looking more carefully you can understand, that is, it is not readable. It seems abuse, and perhaps for lack of having a simpler resource in the language.

Another disadvantage is the performance that is a little lower, although you may not need to worry about that in this case.

Obviously it has the advantage of being shorter like this.

But I wonder if the correct is if the object that has this data should not ensure that it is not null. It doesn’t always work, but in many cases not only does it work, it’s the right thing to do.

The modeling seems wrong too.

3


What I write here first comes from experience I have in the company and then from absorbing what is written in the article (in English) Use Optional correctly is not optional, published in Dzone.

As Maniero himself said, an objective disadvantage of the use of Optional for this purpose is that the performance is worse. Hotspot tries to cushion this by making optimizations in the use of invokeDynamics, but they are only amortizations that will not reach the level of the "purely imperative" version. He demonstrates a subjective reason as to what I must agree: the person/team needs to be used/used to this language.

There are other objective disadvantages to the use of Optional beyond performance. The main one I see: it is not easy to treat intermediate nullity.

While in a world of ifs and elses it is easy to explicitly determine where each mapping occurs and how to behave in cases of nulls, it is an arduous task to do with Optional.

Imagine that you want, for some reason, to determine who was the null element in the string. With the classic way of programming would look something like this:

    int saltosNulidade = -1; // -1 significa que não houve nulos
    if (loja != null) {
        Endereco endereco1 = loja.getEndereco();
        if (endereco1 != null) {
            Estado estado1 = endereco1.getEstado();
            if (estado1 != null) {
                Cidade cidade1 = estado1.getCidade();
                if (cidade1 != null) {
                   nomeCidade = cidade1.getNome();
                } else {
                  saltosNulidade = 3; // conseguiu dar 3 saltos, mas parou num nulo
                }
            } else {
              saltosNulidade = 2; // conseguiu dar 2 saltos, mas parou num nulo
            }
        } else {
          saltosNulidade = 1; // conseguiu dar 1 salto, mas parou num nulo
        }
    } else {
      saltosNulidade = 0; // deu nulo já no primeiro passo
    }

Unfortunately, there is no trivial way to treat each other differently Optional. You can even treat the case of "positive jumps", the logic complementary to how many jumps there was until nullity. But even so, it wastes the use of the Ambdas:

// só para permitir um closure com mudanças, já que o objeto no closure precisa ser
// efetivamente final, mas não seus campos
class IntIndirection {
  int x;
  IntIntidrection(int valorInicial) {
    x = valorInicial;
  }
}

// ...

//
<T, M> Operator<Function<T, M>> retornaMapIncremento(Function<T, M> mapeamento, IntIndirection saltos) {
  return orig -> {
    saltos.x += 1;
    return mapeamento.apply(orig);
  };
};

// ...

// para o caso de loja poder ser nulo também, usar o Optional.ofNullable
// .map(Estado::getCidade).map(Cidade::getNome).orElse("SEM CIDADE CADASTRADA")
Optional<Loja> lojaOptional = Optional.ofNullable(loja);

IntIndirection saltosNulidade = new IntIndirection(0);
lojaOptional.map(retornaMapIncremento(Loja::getEndereco, saltosNulidade))
  .map(retornaMapIncremento(Endereco::getEstado, saltosNulidade))
  .map(retornaMapIncremento(Estado::getCidade, saltosNulidade))
  .map(retornaMapIncremento(Cidade::getNome, saltosNulidade))
  .orElse("SEM CIDADE CADASTRADA");

Notice the effort needed for something simple? And, as the complement algorithm is, it is not strictly equivalent to the one presented earlier. The equivalent of this from here would be the following in imperative version:

int saltosNulidade = 0;
if (loja != null) {
  saltosNulidade++;
  Endereco endereco1 = loja.getEndereco();
  if (endereco1 != null) {
    saltosNulidade++;
    Estado estado1 = endereco1.getEstado();
    if (estado1 != null) {
      saltosNulidade++;
      Cidade cidade1 = estado1.getCidade();
      if(cidade1 != null) {
         saltosNulidade++;
         nomeCidade = cidade1.getNome();
      }
    }
  }
}

However, I must state that most of the "chain processing" that I need to do in production code does not involve elses intermediary. Therefore, although this is an extreme disadvantage in the case of use that practically forces the use of the traditional if-else, she is not extremely common.

An advantage of using mapping through the Optional is to stay one-line, with the explicit exception case. That’s what Maniero brushed on "has the advantage of being shorter like this", but the aesthetic improvement here is somewhat beyond the "shorter you get". It is also more direct (of course, this is something subjective). For example, taking this excerpt from a production code:

lbRiscoValue.setText(Optional.of(cliente)
  .map(Cliente::getRisco)
  .map(Risco::getDsRisco)
  .orElse("Sem resultado"));

In this modeling, the object cliente may not have the field risco, already risco it is guaranteed to have a dsRisco. The existence of cliente this part of the code.

The imperative equivalent of the above, given these above restrictions, is:

String textoLabel = "Sem resultado";
Risco r = cliente.getRisco();
if (r != null) {
  textoLabel = r.getDsRisco();
}

lbRiscoValue.setText(textoLabel);

or alternatively

String textoLabel = null;
Risco r = cliente.getRisco();
if (r != null) {
  textoLabel = r.getDsRisco();
}

lbRiscoValue.setText(textoLabel != null? textoLabel: "Sem resultado");

or alternatively

String textoLabel;
Risco r = cliente.getRisco();
if (r != null) {
  textoLabel = r.getDsRisco();
} else {
  textoLabel = "Sem resultado";
}

lbRiscoValue.setText(textoLabel);

More efficient than the version with Optional? Yes, of course. But the line of code that reflects the desired action (lbRiscoValue.setText) is placed as a detail with less emphasis, while in the version with Optional this becomes more evident.

Before having access to Optional of Java 8, the project I was working on used Java 7, but we were able to use lambda using Retrolambda. For most cases, it was not necessary to use something similar to Optional, but a single-level mapping (as seen above). Therefore, at the time, we created the Toolbox.getIfNotNull. The version with this function would be as follows:

lbRiscoValue.setText(Toolbox.getIfNotNull(cliente.getRisco(), Risco::getDsRisco, "Sem Resultado"));

One more less trivial example. I have an object of the type Verba, that cannot be instantiated. But she knows what types she can be and, depending on her type, she can undergo an implicit conversion through her own methods. For example, Verba.asVerbaFinanceira(), returns a Optional<VerbaFinanceira> completed if, by chance, the Verba is really a VerbaFinanceira. If not, returns a Optional.empty(). May happen of a VerbaFinanceira cause a specific type of pending. That’s how the code looks:

return Optional.ofNullable(verba).flatMap(Verba::asVerbaFinanceira)
    .map(VerbaFinanceira::getTipoBlocPedidoSit)
    .map(TipoBlocPedidoSit::getCdTipoBloc)
    .map(TipoPedidoPendente::getTipoPedidoPendenteFromMnemonico)
    .orElse(null);

The interactive equivalent of this would be (let’s imagine that there is a method isVerbaFinanceira() returning false when the asVerbaFinanceira() returns a Optional.empty() and true otherwise):

TipoPedidoPendente tpp = null;
if (verba != null) {
  if (verba.isVerbaFinanceira()) {
    VerbaFinanceira vf = (VerbaFinanceira) verba;
    TipoBlocPedidoSit tbps = vf.getTipoBlocPedidoSit();
    if (tbps != null) {
      Integer cdTipoBloc = tbps.getCdTipoBloc();
      if (cdTipoBloc != null) {
        tpp = TipoPedidoPendente.getTipoPedidoPendenteFromMnemonico(cdTipoBloc);
      }
    }
  }
}
return tpp;

Note that the value default (in the case null) is initialized in tpp right at the start. If you had another value to enter this pattern, start tpp with this other value would be an error. By the way, exactly because of this error I did not put the return within the most nested level of ifs. If there is another value to be returned (suppose it is the constant TipoPedidoPendente.ALGO), the codes would be the following:

return Optional.ofNullable(verba).flatMap(Verba::asVerbaFinanceira)
    .map(VerbaFinanceira::getTipoBlocPedidoSit)
    .map(TipoBlocPedidoSit::getCdTipoBloc)
    .map(TipoPedidoPendente::getTipoPedidoPendenteFromMnemonico)
    .orElse(TipoPedidoPendente.ALGO);
TipoPedidoPendente tpp = null;
if (verba != null) {
  if (verba.isVerbaFinanceira()) {
    VerbaFinanceira vf = (VerbaFinanceira) verba;
    TipoBlocPedidoSit tbps = vf.getTipoBlocPedidoSit();
    if (tbps != null) {
      Integer cdTipoBloc = tbps.getCdTipoBloc();
      if (cdTipoBloc != null) {
        tpp = TipoPedidoPendente.getTipoPedidoPendenteFromMnemonico(cdTipoBloc);
      }
    }
  }
}
return tpp != null? tpp: TipoPedidoPendente.ALGO;

There are other uses for the Optional in addition to "protect against NPE", which is quoted in the question. I will only brush a little on one of these other uses, without going into too much detail: processing into a function of a non-zero value.

For example, I need to be filled in, in a visual component, which elements are selected from a list. By chance, this screen, in the modeling in which it was created, receives as argument the object to be modified (if it is an object update) or null (new object). The object in question is the usuarioAlcadaAprovacaoDTO.

For reasons of optimizations in GWT, startup time and difficulty in separating into smaller GWT modules of the project, much of the padding of the objects on the screen is done through what we informally call Tripleta, consisting of a code, a text and a classifier, that classifier used to decrease the number of ajax calls via GWT-RPC, allowing a multiplexing of the results

To map this object to who must be filled, I do the following:

Optional.ofNullable(this.usuarioAlcadaAprovacaoDTO)
  .map(UsuarioAlcadaAprovacaoDTO::getUsuarios)
  .map(usuarios -> usuarios.stream().map(p -> new Tripleta(p, Constantes.USUARIO_SISTEMA)).collect(Collectors.toList()))
  .ifPresent(usuariosSelect::setSelectedValues);

Note that I only fill the value in usuariosSelect if the object is updating, if it has filled in which users should I fill in and only after the mapping of Usuario for Tripleta. Supposedly, how getUsuarios returns a collection, it should be ensured that this collection is not null at most empty. Anyway, it’s clear to those who are used to this language that the focus is to call usuariosSelect.setSelectedValues(usuarios), all the rest of the above is just the way to make that call.

The equivalent without Optional (nor Stream, in that case) that would be:

if (this.usuarioAlcadaAprovacaoDTO != null) {
  List<Usuario> usuarios = this.usuarioAlcadaAprovacaoDTO.getUsuarios();

  // vou assumir que não é nulo por questões de razoabilidade
  ArrayList<Tripleta> usuariosAsTripletas = new ArrayList<>();
  for (Usuario usuario: usuarios) {
    usuariosAsTripletas.add(new Tripleta(usuario, Constantes.USUARIO_SISTEMA));
  }
  usuariosSelect.setSelectedValues(usuariosAsTripletas);
}

For those who are used to this language, it is clear that the focus is to call usuariosSelect.setSelectedValues.

Another advantage is the else of ifPresentOrElse (Java 9, which GWT couldn’t stand but I did a broad backport). In it, you do all the necessary mapping and then take one of two available actions:

  • if there is value mapped at the end
  • in the absence of value at the end

Let’s take the case:

lbRiscoValue.setText(Optional.of(cliente)
  .map(Cliente::getRisco)
  .map(Risco::getDsRisco)
  .orElse("Sem resultado"));

To treat the absence of value at the end, could do the following:

Optional.of(cliente)
  .map(Cliente::getRisco)
  .map(Risco::getDsRisco)
  .ifPresentOrElse(
      lbRiscoValue::setText,
      () -> showToast("Sem risco associado ao cliente")
  );

The imperative equivalent would be something like this:

String text = null;
Risco r = cliente.getRisco();

if (r != null) {
  text = r.getDsRisco();
}

if (text != null) {
  lbRiscoValue.setText(text);
} else {
  showToast("Sem risco associado ao cliente");
}

Browser other questions tagged

You are not signed in. Login or sign up in order to post.