CODEWITHSUNDEEP
java
quarks
quarks

Reputation: 35276

Other ways to check for not null in Java

I have a lot of this kind of code in my project:

if (entityRepository.saveEntity(new RemoteEntityBuilder()
   .appId(appId)
   .nameSpace(nameSpace)
   .entityType(entityType)
   .entityId(entityId)
   .blobs(Lists.list(new RemoteBlobBuilder()
      .blobName(blobName)
      .blobStream(new SimpleRemoteInputStream(inputStream))
      .build()))
   .build()) != null) {  
   // Meaning entity was saved
} else {
   // Meaning entity was not saved
}

The saveEntity method returns either NULL (if operation failed) or the object/entity that was saved if the operation was successful. My question is, is there a better way to represent this code with the use of != null for instance:

if(entityRepository.saveEntity(...)) {
}

Or something else.

UPDATE:

The saveEntity method is this

  @Override public RemoteEntity saveEntity(RemoteEntity entity)
      throws NotBoundException, RemoteException {
    RemoteEntities remoteEntities = saveEntities(new RemoteEntity[] {entity});
    return remoteEntities != null ? remoteEntities.entities().stream().findFirst().get() : null;
  }

Here's how it looks now thanks to YCF_L:

  entityRepository.saveEntity(new RemoteEntityBuilder()
                .appId(appId)
                .nameSpace(nameSpace)
                .entityType(entityType)
                .entityId(entityId)
                .blobs(Lists.list(new RemoteBlobBuilder()
                    .blobName(blobName)
                    .blobStream(new SimpleRemoteInputStream(inputStream))
                    .build()))
                .build()).ifPresentOrElse(remoteEntity -> {
              pubSubService.updated(remoteEntity.appId(), remoteEntity.nameSpace(),
                  remoteEntity.entityType(), remoteEntity.entityId());
              setStatus(Status.SUCCESS_CREATED);
            }, () -> {
              setStatus(Status.CLIENT_ERROR_BAD_REQUEST);
            });

Here's how the code looks in the IDE (looks pretty clean to me):

enter image description here

Upvotes: 1

Views: 88

Answers (2)

Youcef LAIDANI
Youcef LAIDANI

Reputation: 59978

I would use Optional in your case :

public Optional<RemoteEntity> saveEntity(RemoteEntity entity) throws NotBoundException, RemoteException {
    RemoteEntities remoteEntities = saveEntities(new RemoteEntity[]{entity});
    return remoteEntities.entities().stream()
            .findFirst();
}

and then :

if(entityRepository.saveEntity(...).isPresent()) {
   ...       
}

In fact you have many choices with Optional, you can use ifPresent also :

entityRepository.saveEntity(...)
    .ifPresent(r -> ..)

Or throw an exception:

entityRepository.saveEntity(...)
    .orElseThrow(() -> ..)

Upvotes: 4

Dave Costa
Dave Costa

Reputation: 48121

What is "better" may be a matter of opinion.

Given your example, the way to achieve that would be to create another method that calls saveEntity() and returns true or false. (I do wonder why saveEntity() doesn't throw an exception if its operations fails -- that would be more normal in my experience.)

If you simply don't like that the comparison is hard to spot, you might reverse the order:

if (null != entityRepository.saveEntity(...))

I would probably move the call outside of the if entirely, as I find side effects in conditionals potentially confusing.

RemoteEntity myEntity = entityRepository.saveEntity(...)
if (myEntity != null) ...

Upvotes: 1

Related Questions