Is there a way to minimise if and if-else condition

Question

I have written a code which is working fine but I'am using too many If and else-if conditions. Is there a way to minimise it? Based on the integer values shippingStatus, invoiceStatus and paymentStatus values should change.

    int qtyOrdered = dynamic integer values;
    int qtyShipped = dynamic integer value;
    int qtyReturned = dynamic integer values;
    int qtyInvoiced = dynamic integer values;

        OrderShippingStatus shippingStatus = shippingStatus(qtyOrdered,qtyShipped,qtyReturned);
        OrderInvoicingStatus invoiceStatus = invoiceStatus(qtyOrdered,qtyInvoiced,qtyReturned);
        OrderPaymentStatus paymentStatus =paymentStatus(salesOrder.getAmountPaid(),totalAmountAfterEvent);

private OrderPaymentStatus paymentStatus(BigDecimal amountPaid, BigDecimal totalAmountAfterEvent) {
        if (MathUtils.isFirstLessThanSecond(totalAmountAfterEvent, amountPaid)) {
            return OrderPaymentStatus.FULLY_PAID;
        } else if (MathUtils.areEqual(totalAmountAfterEvent, BigDecimal.ZERO)) {
            return OrderPaymentStatus.NOT_APPLICABLE;
        } else if (MathUtils.isFirstLessThanSecond(amountPaid, totalAmountAfterEvent) && (MathUtils.isFirstLessThanSecond(amountPaid, totalAmountAfterEvent))) {
            return OrderPaymentStatus.PARTIALLY_PAID;
        } else if (MathUtils.isFirstLessThanSecond(amountPaid, totalAmountAfterEvent) && (MathUtils.isFirstLessThanSecond(totalAmountAfterEvent, amountPaid) || (MathUtils.areEqual(totalAmountAfterEvent, amountPaid)))) {
            return OrderPaymentStatus.PARTIALLY_PAID;
        } else {
            return salesOrder.getPaymentStatus();
        }
    }

    private OrderInvoicingStatus invoiceStatus(int qtyOrdered, int qtyInvoiced, int qtyReturned) {
        if (qtyOrdered == qtyInvoiced && qtyInvoiced > qtyReturned) {
            return OrderInvoicingStatus.FULLY_INVOICED;
        } else if (qtyOrdered == qtyInvoiced && qtyInvoiced == qtyReturned) {
            return OrderInvoicingStatus.NOT_APPLICABLE;
        } else if (qtyOrdered > qtyInvoiced && qtyInvoiced > qtyReturned) {
            return OrderInvoicingStatus.PARTIALLY_INVOICED;
        } else if (qtyOrdered > qtyInvoiced && qtyInvoiced == qtyReturned) {
            return OrderInvoicingStatus.NOT_INVOICED;
        } else {
            return salesOrder.getInvoiceStatus();
        }
    }

    private OrderShippingStatus shippingStatus(int qtyOrdered, int qtyShipped, int qtyReturned) {
        if (qtyOrdered == qtyShipped && qtyShipped >= qtyReturned) {
            return OrderShippingStatus.FULLY_SHIPPED;
        } else if (qtyOrdered > qtyShipped && qtyShipped > qtyReturned) {
            return OrderShippingStatus.PARTIALLY_SHIPPED;
        } else if (qtyOrdered > qtyShipped && qtyShipped == qtyReturned) {
            return OrderShippingStatus.NOT_SHIPPED;
        } else {
            return salesOrder.getShippingStatus();
        }
    }

may be a `Map, ShipStatus>` where `Context` has all the fields you need to be tested — Eugene, May 02 '19 at 08:24
Since each block of IFs only writes to a single variable, and always writes it, it's easy to split it into three extra methods. This doesn't make the code smaller but it makes it much more readable (which is more important) — Mark Jeronimus, May 02 '19 at 08:26

score 2 · Answer 1 · answered May 02 '19 at 09:13

If you are a fun of java 8 and lambda expresions, you can do:

1.Use an existing Functional Interface or define a new one that match your requirements

@FunctionalInterface
public interface TriPredicate<A, B, C> {

    boolean test(A a, B b, C c);

}

2.Create an enum that contain the condition. Implement a method that return the element that match the enum predicate

import java.util.Arrays;
import java.util.Optional;

public enum ShiStatus {
    FULLY_SHIPPED((qtyOrdered, qtyShipped, qtyReturned) -> qtyOrdered.equals(qtyShipped) && qtyShipped >= qtyReturned),
    PARTIALLY_SHIPPED((qtyOrdered, qtyShipped, qtyReturned) -> qtyOrdered > qtyShipped && qtyShipped > qtyReturned),
    NOT_SHIPPED((qtyOrdered, qtyShipped, qtyReturned) -> qtyShipped != 0.0 && qtyShipped.equals(qtyReturned));

    private TriPredicate<Double, Double, Double> predicate;

    ShiStatus(TriPredicate<Double, Double, Double> predicate) {
        this.predicate = predicate;
    }

    public TriPredicate<Double, Double, Double> getPredicate() {
        return predicate;
    }

    public static Optional<ShiStatus> getStatus(Double qtyOrdered, Double qtyShipped, Double qtyReturned) {
        return Arrays.stream(ShiStatus.values())
                .filter(shiStatus -> shiStatus.getPredicate().test(qtyOrdered, qtyShipped, qtyReturned))
                .findFirst();
    }

}

3.Use the enum method to get the status base on the enum condition

    @Test
    public void testEnum() {
        ShiStatus shippingStatus = ShiStatus.NOT_SHIPPED; // salesOrder.getShippingStatus()

        Assert.assertEquals(ShiStatus.PARTIALLY_SHIPPED, ShiStatus.getStatus(3D, 2D, 1D).orElse(shippingStatus));
        Assert.assertEquals(ShiStatus.NOT_SHIPPED, ShiStatus.getStatus(1D, 2D, 3D).orElse(shippingStatus));
    }

Instead of a tripredicate you could just define `getStatus` abstract in the enum and implement it in each enum element. — Mark Jeronimus, May 02 '19 at 09:42
Also, document properly that the order of the enums MATTERS, as the order is used for iterating `values()`. Using strict enum ordering goes against enum best practices, but whatever. — Mark Jeronimus, May 02 '19 at 09:44

Maximilian Horn · Answer 2 · 2019-05-02T10:06:36.640

As @Mark Jeronimus already mentioned in the comments you could use three seperate methods to make it more readable. Since you are comparing different aspects, a switch() function wouldnt make that much sense. Also you could use more white spaces... My suggestion:

Make this a function:

if (qtyOrdered == qtyShipped && qtyShipped >= qtyReturned) {
    shippingStatus = ShiStatus.FULLY_SHIPPED;
} 
else if (qtyOrdered > qtyShipped && qtyShipped > qtyReturned) {
    shippingStatus = ShiStatus.PARTIALLY_SHIPPED;
} 
else if (qtyOrdered > qtyShipped && qtyShipped == qtyReturned) {
    shippingStatus = ShiStatus.NOT_SHIPPED;
} 
else {
    shippingStatus = salesOrder.getShippingStatus();
}

Make this a second function:

if (qtyOrdered == qtyInvoiced && qtyInvoiced > qtyReturned) {
    invoiceStatus = ShiStatus.FULLY_INVOICED;
} 
else if (qtyOrdered == qtyInvoiced && qtyInvoiced == qtyReturned) {
    invoiceStatus = InvStatus .NOT_APPLICABLE;
} 
else if (qtyOrdered > qtyInvoiced && qtyShipped > qtyReturned) {
    invoiceStatus = InvStatus .PARTIALLY_INVOICED;
} 
else if (qtyOrdered > qtyInvoiced && qtyShipped == qtyReturned) {
    invoiceStatus = ShiStatus.NOT_INVOICED;
} 
else {
    invoiceStatus = salesOrder.getInvoiceStatus();
}

Make this your third function:

if (MathUtils.isFirstLessThanSecond(totalAmountAfterEvent, salesOrder.getAmountPaid())) {
    paymentStatus = PayStatus .FULLY_PAID;
} 
else if (MathUtils.areEqual(totalAmountAfterEvent, BigDecimal.ZERO)) {
    paymentStatus = PayStatus .NOT_APPLICABLE;
} 
else if (MathUtils.isFirstLessThanSecond(salesOrder.getAmountPaid(), totalAmountAfterEvent) && (MathUtils.isFirstLessThanSecond(salesOrder.getAmountPaid(), totalAmountAfterEvent))) {
    paymentStatus = PayStatus .PARTIALLY_PAID;
} 
else if (MathUtils.isFirstLessThanSecond(salesOrder.getAmountPaid(), totalAmountAfterEvent) && (MathUtils.isFirstLessThanSecond(totalAmountAfterEvent, salesOrder.getAmountPaid()) || (MathUtils.areEqual(totalAmountAfterEvent, salesOrder.getAmountPaid())))) {
     paymentStatus = PayStatus .PARTIALLY_PAID;
} 
else {
     paymentStatus = salesOrder.getPaymentStatus();
}

____________________________________________________________

Alternatively you could make every if thing (?) a int value:

private static int conditions() {
     if(your_first_statement) {
         return 0;
     }
     else if(your_second_statement) {
         return 1;
     } 
     // continue with all your if statements and put this method way down in your programm

So your main function would be:

  private static void whatever_function_youre_in() {
      int condition = conditions();
      switch(condition) {
      case 0: {
          //your code from the first if statement
      }
      case 1: {
          // your code from the second if statement
      }
      } // continue with all your if conditions
   }    // your else conditions could be the default case from switch (maybe)

EDIT

Since it wasnt clear what i meant with the 3 functions:

In the code the creator gave you need to pack them into voids an execute them like:

function1();
function2();
function3();

After that continue with my switch case.

However keep in mind that you have to execute the conditons() first and declare all your int values to the whole class. For example:

 private static int your_int;
 private static int your_second_int;
 // and so on...

EDIT2

And for the poor fellas out there (including the creator) here is the full code:

public class your_class() {
private static int conditions() {
if (qtyOrdered == qtyShipped && qtyShipped >= qtyReturned) {
       return 0;
    } 
    else if (qtyOrdered > qtyShipped && qtyShipped > qtyReturned) {
        return 1;
    } 
    else if (qtyOrdered > qtyShipped && qtyShipped == qtyReturned) {
        return 2;
    } 
    else {
        return 3;
    }
    if (qtyOrdered == qtyInvoiced && qtyInvoiced > qtyReturned) {
        return 4;
    } 
    else if (qtyOrdered == qtyInvoiced && qtyInvoiced == qtyReturned) {
        return 5;
    } 
    else if (qtyOrdered > qtyInvoiced && qtyShipped > qtyReturned) {
        return 6;
    } 
    else if (qtyOrdered > qtyInvoiced && qtyShipped == qtyReturned) {
        return 7;
    } 
    else {
        return 8;
    }
    if (MathUtils.isFirstLessThanSecond(totalAmountAfterEvent, salesOrder.getAmountPaid())) {
        return 9;
    } 
    else if (MathUtils.areEqual(totalAmountAfterEvent, BigDecimal.ZERO)) {
        return 10;
    } 
    else if (MathUtils.isFirstLessThanSecond(salesOrder.getAmountPaid(), totalAmountAfterEvent) && (MathUtils.isFirstLessThanSecond(salesOrder.getAmountPaid(), totalAmountAfterEvent))) {
        return 11;
    } 
    else if (MathUtils.isFirstLessThanSecond(salesOrder.getAmountPaid(), totalAmountAfterEvent) && (MathUtils.isFirstLessThanSecond(totalAmountAfterEvent, salesOrder.getAmountPaid()) || (MathUtils.areEqual(totalAmountAfterEvent, salesOrder.getAmountPaid())))) {
        return 12;
    } 
    else {
        return 13;
    }
}

private static void main(Strings[] args) {
int conditions = conditions();
switch(condition) {
case 0: {
    shippingStatus = ShiStatus.FULLY_SHIPPED;
}
case 1: {
    shippingStatus = ShiStatus.PARTIALLY_SHIPPED;
}
// all your other cases
}
}

I think you can understand it now better and complete the rest of switch. Note: You have to make all your dynamic int´s private static int in your class available.

how to use those functions after creating 3 , could you please show one example? — eshwar chettri, May 02 '19 at 09:06
@eshwar chettri I would declare your int in your class as private static. Then u dont need to do the parameter stuff for every function and every if condition. After that you just call them like: function1() {} and function2{} and function3{}. After that you go to the switch case. Be clear that you need to do the conditons() first... — Maximilian Horn, May 02 '19 at 09:10
in conditions() should i write all the if and else-if condition and return 0,1,2,3 so on? — eshwar chettri, May 02 '19 at 09:16
@eshwarchettri yes. It is more code but way more readable. Just put conditions() as your last function in your program so no one sees it — Maximilian Horn, May 02 '19 at 09:27
what comes in "your_first_statement" if(your_first_statement) — eshwar chettri, May 02 '19 at 09:31
I'd use your IDE's 'extract method' functionality, and you'll see that the method has one of your enums as it's return type and the body of each IF becomes a return statement. — Mark Jeronimus, May 02 '19 at 09:48
@KBS_Shabib can you give permission to cannibalize your post and rewrite it to how I imagine the best '3 methods' solution looks? (you can always rollback my edit if you don't like it.) — Mark Jeronimus, May 02 '19 at 09:50
@MarkJeronimus how do i do that? Also i am currently writing the whole program. just give me some time :) but you can if i know how — Maximilian Horn, May 02 '19 at 09:52
@eshwarchettri I put almost your whole solution in there. Just complete switch and maybe make the main a while function so you get all possibiities and dont break after one. — Maximilian Horn, May 02 '19 at 10:02
@Markjeronimus provide your own then. Feel free to use parts of my code If you want. But don't tell them I am bad or something. Many thanks — Maximilian Horn, May 02 '19 at 10:20
It seems OP already split the methods in the way I imagined the solution to be. — Mark Jeronimus, May 02 '19 at 11:18

Is there a way to minimise if and if-else condition

2 Answers2