Is there a better way to write this if statements?-CodePudding

I started recently as a developer and I am still struggling a bit with the way I write my code.

Is there a better way to write this two if-statements? How would you write it and why?

Java code:

@Override
@Transactional
public void deleteItem(final ConfigurationType type, final long itemId, final boolean force) {
    this.applicationNameUtils.throwOnInvalidApplication(type.getApplication());
    final ConfigurationItemModel item =
        this.configurationItemRepository.findByApplicationAndTopicAndId(type.getApplication(), type.getTopic(), itemId)
            .orElseThrow(() -> new ResourceNotFoundException(itemId, "Configuration Item"));
    if (Boolean.TRUE.equals(item.getContentModificationOnly()) && Boolean.FALSE.equals(force)) {
        throw new ContentModificationOnlyException("Configuration Item cannot be deleted");
    }
    if ((Boolean.TRUE.equals(item.getContentModificationOnly()) || Boolean.FALSE.equals(item.getContentModificationOnly())) && Boolean.TRUE.equals(force)) {
        this.assignmentService.deleteAssignmentsByItem(item);
        this.configurationInstanceRepository.deleteByItem(item);
        this.configurationItemRepository.deleteById(itemId);
    }
}

I am not sure if I can somehow combine this two in a if-else.

CodePudding user response：

It looks like you don't care about item.getContentModificationOnly() is true or false in the second if-statement since your code is (Boolean.TRUE.equals(item.getContentModificationOnly()) || Boolean.FALSE.equals(item.getContentModificationOnly()). So if your logic is right I suggest you code like this:


if (fore) {
    this.assignmentService.deleteAssignmentsByItem(item);
    this.configurationInstanceRepository.deleteByItem(item);
    this.configurationItemRepository.deleteById(itemId);
} else if (Boolean.TRUE.equals(item.getContentModificationOnly()) {
    throw new ContentModificationOnlyException("Configuration Item cannot be deleted");
}

CodePudding user response：

First if condition

 if (item.getContentModificationOnly() && !force) {

Second If condition

if ((item.getContentModificationOnly() || !item.getContentModificationOnly()) && force) {

The below code will always return true

(item.getContentModificationOnly() || !item.getContentModificationOnly())

so modify second if stmnt to just

if (force){

CodePudding user response：

Depends on the return type item.getContentModificationOnly(). If it's Boolean, than the second statement can be reduced to

if(item.getContentModificationOnly() != null && force)

If the return type of item.getContentModificationOnly() is boolean, than the statement can be reduced to

if(force)

and the answer of @LiLittleCat above if correct.