CODEWITHSUNDEEP
javagenericscasting
Jim
Jim

Reputation: 4405

Can I avoid the casting to T for this case?

I have a function with the following structure:

public class TestOrder {

    enum Status {
        CANCELLED,
        EXECUTED
    }
    
    static class Data {
        Status status;        
    }

    private  <T extends Order> void process(List<Data> dataList, List<T> currentUpdates) {
    List<T> orders = new ArrayList<>();
    for(Data d: dataList) {
        Order order = createOrder(d);
        if(order != null) {
            orders.add((T) order); // <- warning "Unchecked cast Order to T
        }
    }

    // processing
    currentUpdates.clear();
    currentUpdates.addAll(orders);
}

    private Order createOrder(Data data) {
        if(data.status == Status.EXECUTED) {
            // processing
            return new ExecutedOrder();  // implements of Order
        }
        if(data.status == Status.CANCELLED) {
            // processing
            return new CancelledOrder();  // implements of Order
        }

        return null;
    }
}



public interface Order {
}

public class CancelledOrder implements Order {
}

public class ExecutedOrder implements Order {
}

Is there a way to avoid that warning? The actual object is always a subtype of Order so I was wondering if either I could avoid that warning or there is problem I am overlooking.

Example of calling the method (trivial for clarity):

public static void main(String[] args) {
        List<ExecutedOrder> executedOrders = new ArrayList<>();
        process(getDataList(), executedOrders);
    }

    private static List<Data> getDataList() {
        return new ArrayList<>();
    }

Upvotes: 1

Views: 87

Answers (4)

Henry Twist
Henry Twist

Reputation: 5980

There isn't a way to avoid the warning whilst your code is in its current form. Consider this simplified scenario:

public static void main(String[] args) {
    
    List<ExecutedOrder> executedOrders = new ArrayList<>();
    process(executedOrders);
    
    ExecutedOrder executedOrder = executedOrders.get(0);
}

private static <T extends Order> void process(List<T> currentUpdates) {
    
    currentUpdates.add((T) createOrder());
}

private static Order createOrder() {

    return new CancelledOrder();
}

Here, there will be a ClassCastException thrown when accessing elements of your list. So the reason the compiler is telling you that the cast is unsafe, is because it is.

Although you mentioned in the comments that your Data arguments would guarantee the correct orders being added to the list, the compiler has no way to verify this. This isn't necessarily a problem, unsafe casts exist in a great many places, but you have to be aware that what you're doing is fundamentally unsafe.

I think that shmosel's answer gives the best alternative, so it might be worth rethinking your approach to something similar.

Upvotes: 3

NoDataFound
NoDataFound

Reputation: 11949

In addition to other answers, if you wish to "fly" away the cast:

private <T extends Order> void process(
  List<Data> dataList,  
  Class<T> orderType,
  List<T> currentUpdates
) {
 List<T> orders = new ArrayList<>();
 for (Data d: dataList) {
   T order = orderType.cast(createOrder(d)); // check are no longer required
   if (order != null) {
     orders.add(order); 
   }
}

But then again, the List<T> is more than problematic here: without the class (and therefore the cast), you could add a CancelledOrder in a List<ExecutedOrder> and this would fail when you access it.

I would personally stick to List<Order> and remove the generic <T> as it does not make sense.

Especially if you need to pass the type (using a class in such case works fine in some case, not all case):

   List<ExecutedOrder> currentUpdates = ...;
   process(dataList, ExecutedOrder.class, currentUpdates);

You should perhaps see why you are trying to use a List<ExecutedOrder> or List<CancelledOrder> in the first place.

Upvotes: 1

shmosel
shmosel

Reputation: 50716

It's hard to offer an ideal solution based on sample code, but given what you've provided, it might be worth refactoring Data and Status to be type-aware:

public class TestOrder {

    static abstract class Status<T extends Order> {
        public static final Status<CancelledOrder> CANCELLED = new Status<>() {
            @Override
            public CancelledOrder createOrder() {
                return new CancelledOrder();
            }
        };

        public static final Status<ExecutedOrder> EXECUTED = new Status<>() {
            @Override
            public ExecutedOrder createOrder() {
                return new ExecutedOrder();
            }
        };

        public abstract T createOrder();
    }
    
    static class Data<T extends Order> {
        Status<T> status;        
    }

    private <T extends Order> void process(List<Data<T>> dataList, List<T> currentUpdates) {
        List<T> orders = new ArrayList<>();
        for(Data d: dataList) {
            T order = d.status.createOrder();
            if(order != null) {
                orders.add(order);
            }
        }
    
        // processing
        currentUpdates.clear();
        currentUpdates.addAll(orders);
    }

}

Upvotes: 3

Jack
Jack

Reputation: 133567

Well, if the object is always a subtype of Order I don't see the requirement of having List<T extends Order> in the first place when you could have List<Order>.

You are losing informations about a type by using a Order createOrder method and then you want it back. This is an approach which makes no sense if there's not a specific requirement.

Upvotes: 2

Related Questions