AnswerBun.com

Ensuring if an object is Assignable from a class type

Code Review Asked by Sourabh on October 28, 2020

I am trying to ensure type in the below method. How to reduce its cognitive complexity to below 15 from the current 27? It looks more readable as it is? Is there a way to break this down to make it more readable?

Shifting some of the checks to separate method will be one way but is there a better way?

private Object ensureType(Class<?> expectedClass, Object value) {
    if (value == null)
        return null;
    Class<?> valueClass = value.getClass();
    if (expectedClass.isAssignableFrom(valueClass))
        return value;

    if (expectedClass.isAssignableFrom(String.class)) {
        return value.toString();
    }
    if (expectedClass.isAssignableFrom(Date.class)) {
        if (Number.class.isAssignableFrom(valueClass)) {
            return new Date(((Number) value).intValue());
        }
        try {
            return formatter.parse(value.toString());
        } catch (ParseException e) {
            throw new ClassCastException("argument " + value + " of type " + valueClass + " cannot be converted to "
                    + expectedClass + ":" + e);
        }
    }
    if (expectedClass.isAssignableFrom(Integer.class)) {
        if (Number.class.isAssignableFrom(valueClass))
            return ((Number) value).intValue();
        return Integer.parseInt(value.toString());
    }
    if (expectedClass.isAssignableFrom(Double.class)) {
        if (Number.class.isAssignableFrom(valueClass))
            return ((Number) value).doubleValue();
        return Double.parseDouble(value.toString());
    }
    if (expectedClass.isAssignableFrom(Float.class)) {
        if (Number.class.isAssignableFrom(valueClass))
            return ((Number) value).floatValue();
        return Float.parseFloat(value.toString());
    }
    if (expectedClass.isAssignableFrom(Short.class)) {
        if (Number.class.isAssignableFrom(valueClass))
            return ((Number) value).shortValue();
        return Short.parseShort(value.toString());
    }
    if (expectedClass.isAssignableFrom(Byte.class)) {
        if (Number.class.isAssignableFrom(valueClass))
            return ((Number) value).byteValue();
        return Byte.parseByte(value.toString());
    }
    if (expectedClass.isAssignableFrom(Long.class)) {
        if (Number.class.isAssignableFrom(valueClass))
            return ((Number) value).longValue();
        return Long.parseLong(value.toString());
    }
    if (expectedClass.isAssignableFrom(Boolean.class)) {
        return Boolean.parseBoolean(value.toString());
    }

    throw new ClassCastException(
            "argument " + value + " of type " + valueClass + " cannot be converted to " + expectedClass);
}

PS: I am moving this question from Stackoverflow to CR as it was suggested in the comments.

One Answer

private Object ensureType(Class<?> expectedClass, Object value) {

Your names could be slightly better, given that this function does not only ensure something, but also converts it.

private Object convert(Class<?> targetClass, Object value) {

    throw new ClassCastException(
            "argument " + value + " of type " + valueClass + " cannot be converted to " + expectedClass);

Given that the function converts, I find the ClassCastException inappropriate. An IllegalStateException or IllegalArgumentException might be better fitting.


What you're doing here is complex, and there is not really an easier way to do it either. You have n classes that you want to map to m classes, so the complexity will always be there in one way or another. The best thing you can do is make it more readable. For example with extracting the converting parts into functions:

    // ...
    
    if (targetClass.isAssignableFrom(String.class)) {
        return convertToString(value);
    }

    if (targetClass.isAssignableFrom(Date.class)) {
        return convertToDate(value);
    }
    
    if (targetClass.isAssignableFrom(Integer.class)) {
        return convertToInteger(value);
    }
    
    // ...

That is as readable as it gets, in my opinion.

If you like classes, you can always extract the conversion into a single class for each mapping, and then iterate over that:

public interface Converter {
    public Class<?> getTargetType();
    public Object convert(Object value);
}

// ...

List<Converter> converters = new ArrayList<>();
converters.add(new StringConverter());
converters.add(new DateConverter());
converters.add(new IntegerConverter());
// ...

// ...

for (Converter converter : converters) {
    if (targetClass.isAssignableFrom(converter.getTargetType())) {
        return converter.convert(value);
    }
} 

This, however, has the downside to split everything over many classes. A possible remedy to that might be to use single functions in the same class, like this:

HashMap<Class<?>, Function<Object, Object>> converters = new HashMap<>();
converters.put(String.class, this::convertToString);
converters.put(Date.class, this::convertToDate);
converters.put(Integer.class, this::convertToInteger);
// ...

// ...

for (Entry<Class<?>, Function<Object, Object>> converterEntry : converters.entrySet()) {
    if (targetClass.isAssignableFrom(converterEntry.getKey())) {
        return converterEntry.getValue().apply(value);
    }
} 

That might be one of the easier readable and maintainable variants.

There is another, completely insane option, actually, which I feel to need to point out. You can use the above setup, with a method for every conversion, and reflection to build this list dynamically. Something along these lines:

private Integer convertToInteger(Object value);
private Date convertToDate(Object value);
private String convertToString(Object value);

// Pseudo-Code follows:

for (Method method : getClass().getDeclaredMethods()) {
    if (method.getName().startsWith("convert")) {
        if (targetValue matches method return type
                && value matches first method parameter) {
            return method.invoke(this, value);
        }
    }
}

But that should be an absolute last resort, as it is not quite easy to maintain without knowing what you've got at your hands there.

Correct answer by Bobby on October 28, 2020

Add your own answers!

Related Questions

Sudoku Game with automatically generated Sudokus

2  Asked on January 9, 2021 by philipp-wilhelm

   

Celsius and Fahrenheit Converter in JavaScript

2  Asked on January 7, 2021 by mohammad-abu-sahadat

 

typescript : clean code remove or decrease alot if’s

1  Asked on January 5, 2021 by gabriel-costa

 

Doubly Linked List Data Structure ADT in C++

2  Asked on January 5, 2021 by guy-on-the-internet

         

Simple Router class

1  Asked on January 3, 2021 by samayo

         

http url connection with kotlin on android

0  Asked on December 29, 2020

   

Calculating a page size to execute a subset of jobs at a time

3  Asked on December 29, 2020 by user93068

 

Decluttering quiz game

0  Asked on December 29, 2020 by kue

 

Custom include? method for a substring

0  Asked on December 23, 2020 by michael-zech

   

Avoiding Python tricks to find anagrams

4  Asked on December 21, 2020 by shubhamprashar

   

Return correct path if windows or linux

1  Asked on December 17, 2020 by protractornewbie

 

What good practice to use for input data with arrays?

1  Asked on December 14, 2020 by vladimirov

   

Scraping reddit using Python

0  Asked on December 14, 2020 by user00257

   

Ask a Question

Get help from others!

© 2022 AnswerBun.com. All rights reserved. Sites we Love: PCI Database, MenuIva, UKBizDB, Menu Kuliner, Sharing RPP, SolveDir