I want to validate below condition but something is going wrong with my if condition and returning invalid results. My validations are: Either productId or productAltID can have value or both can be null If both productId and productAltID are null, then productSellDate and productReturnDate must have value.

If productSellDate and productReturnDate are null, then productId or productAltID should have value.

Please find my code below, I'm getting incorrect result not sure what I'm messing up here:

public class validate {

    public static void main(String[] args) {

        String productId = null;
        String productAltID = "adfafadsf";
        Date productSellDate = null;
        Date productReturnDate = new Date();

        if (productId != null || productAltID != null || productSellDate !=null && productReturnDate != null) {
            System.out.println("validation success");
        } else {
            System.out.println("validation failed");
        }

    }
}

// Valid Scenarios

Combination 1: Valid Scenario
        String productId = null;
        String productAltID = null;
        Date productSellDate = new Date();
        Date productReturnDate = new Date();


Combination 2: Valid Scenario
        String productId = null;
        String productAltID = "3432fefsf";
        Date productSellDate = new Date();
        Date productReturnDate = new Date();

Combination 3: Valid Scenario
        String productId = "sdf3234234324";
        String productAltID = "3432fefsf";
        Date productSellDate = null;
        Date productReturnDate = null;

Combination 4: Valid Scenario
        String productId = null;
        String productAltID = "3432fefsf";
        Date productSellDate = null;
        Date productReturnDate = null;

CodePudding user response：

Let's put it like this:

public static boolean isValid(
    String productId, String productAltId, Date sellDate, Date returnDate
) {
    if (null == productId && null == productAltId) {
        return null != sellDate && null != returnDate; // dates MUST have value
    }
    // here productId or productAltId have value, no need to check dates
    return true;
}

Tests:

System.out.println(isValid(null, null, new Date(), new Date())); // true
System.out.println(isValid(null, "ab", new Date(), new Date())); // true
System.out.println(isValid("cd", "ab", null, null));             // true
System.out.println(isValid("cd", null, null, null));             // true

CodePudding user response：

As noted, the operator && binds tighter than || so the logic wasn't doing what you wished it was. See Operators page of the Oracle tutorial.

A little Literate Programming goes a long way to avoiding this sort of thing.

boolean hasAnId = productId != null || productAltID != null;
boolean hasDates = productSellDate != null && productReturnDate != null;
if ( hasAnId || hasDates ) ...

CodePudding user response：

tl;dr

Stream
.of( productId , productAltID )
.anyMatch( Objects :: nonNull )              // One or more ID fields have a value.
||                                           // … or …
Stream
.of( productSellDate , productReturnDate )
.allMatch( Objects :: nonNull )              // All dates have a value.

Details

The Answer by drekbour seems correct.

Objects.nonNull

In addition, I would suggest using Objects.nonNull & Objects.isNull for easier reading.

Also, I would use more descriptive variable naming.

boolean atLeastOneIdHasValue = Objects.nonNull( productId ) || Objects.nonNull( productAltID ) ;  // One or both ID fields have a value.
boolean bothDatesHaveValue = Objects.nonNull( productSellDate ) && Objects.nonNull( productReturnDate ) ;  // Both dates have a value.
boolean valid = ( atLeastOneIdHasValue || bothDatesHaveValue ) ;

Streams

Another approach uses streams.

Make a stream of your variables:

Stream.of( productId , productAltID )

Then tally if any or all match our criterion.

• Call Stream#anyMatch to see whether any elements of this stream match the provided predicate.
• Call Stream#allMatch to see whether all elements of this stream match the provided predicate.

In our case, the predicate is simply a call to Objects.nonNull.

boolean atLeastOneIdHasValue = Stream.of( productId , productAltID ).anyMatch( x -> Objects.nonNull( x ) ) ;  // One or more ID fields have a value.
boolean bothDatesHaveValue = Stream.of( productSellDate , productReturnDate ).allMatch( x -> Objects.nonNull( x ) ) ;  // All dates have a value.
boolean valid = ( atLeastOneIdHasValue || bothDatesHaveValue ) ;

We can shorten that code by using a method reference as our predicate: Objects :: nonNull.

boolean atLeastOneIdHasValue = Stream.of( productId , productAltID ).anyMatch( Objects :: nonNull ) ;  // One or more ID fields have a value.
boolean bothDatesHaveValue = Stream.of( productSellDate , productReturnDate ).allMatch( Objects :: nonNull ) ;  // All dates have a value.
boolean valid = ( atLeastOneIdHasValue || bothDatesHaveValue ) ;

See this code run live at IdeOne.com.

Avoid Date class

By the way, never use either Date class. Those are now legacy, part of the terrible date-time classes that were years ago supplanted by the modern java.time classes defined in JSR 310.

• For a date only (year-month-day), use LocalDate.
• For a moment as seen in UTC (an offset of zero hours-minutes-seconds), use Instant.
• For a moment as seen in a particular time zone, use ZonedDateTime.