2

Been migrating over some legacy code and I came across this.

@Getter
@Setter
public class CollectedData
{
    SkillResponse skills;
    TrendingResponse storyMatch;
    Map<String, String> slotData;
    Map<String, String> slotDataSecondSource;
    Boolean hasSlots;
    Boolean hasSlotsSecondSource;
    KnowledgeRequest request;
}

Since I've been using java 8 and accustomed to streams, I started to restructure this response class as ..

@Getter
@Setter
public class CollectedData
{
    List<DataSupplierResponse> dataSupplierResponses;
    Metadata metadata;
}

Where DataSupplierResponse was to be a defined interface like so..

public interface DataSupplierResponse<T>
{
    DataSupplierType getDataSupplierType();

    T getSupplierResponse();
}

Implementation Example:

public class DataSupplierResponseImpl implements DataSupplierResponse<TrendingResponse>
{
    private TrendingResponse mTrendingResponse;

    public DataSupplierResponseImpl(
        TrendingResponse trendingResponse)
    {
        mTrendingResponse = trendingResponse;
    }

    @Override
    public DataSupplierType getDataSupplierType()
    {
        return DataSupplierType.TRENDING_STORY;
    }

    @Override
    public TrendingResponse getSupplierResponse()
    {
        return mTrendingResponse;
    } 
}

The goal is to run certain predicates depending on the CollectedData.

Optional<DataSupplierResponse> first = data.getDataSupplierResponses().stream()
                .filter(res -> res.getDataSupplierType().equals(DataSupplierType.TRENDING_STORY))
                .findFirst();

This would need a cast in order to get the right object. It returns Object

TrendingResponse match = first.get().getSupplierResponse();

Thus when I started refactoring, I assumed to solve this issue of data being available by creating the generic interface that returns different data. To make this code work, I would have to cast the return object of getSupplierResponse which defeats the purpose of using generics. I need to make this Data Carrier object as clean and beautiful as possible for my own sake. Any ideas how I should structure these classes, and/or how to use generics to solve this problem.

EDIT: I know the StackOverflow community likes to enforce objective, concrete answers but where else to go to for design questions?

Jacob G.
  • 28,856
  • 5
  • 62
  • 116
sizzle
  • 2,883
  • 2
  • 13
  • 10

2 Answers2

2

You have to specify the List in CollectedData also with generics. E.g:

List<DataSupplierResponse> dataSupplierResponse;

should actually be:

List<DataSupplierResponse<YourType>> dataSupplierResponse;

where YourType corresponds to the type of the response. That is because when using a RawType (a generic class without actually specifiying a generic) all Generic information for that class is eliminated. That's why it is returning Objects and you have to manually cast it.

Lino
  • 19,604
  • 6
  • 47
  • 65
  • 1
    1+, OP added the generics in one part of the code, but not the other... – Eugene Mar 02 '18 at 09:43
  • @Eugene exactly. though what I don't know is, if all elements in the list have the same return type. If they don't then the answer will be quite different and a lot more difficult – Lino Mar 02 '18 at 09:46
  • 1
    I did not go into the details of the question too much to be honest, so I'll trust you here – Eugene Mar 02 '18 at 09:47
1

Unless used in other places, I'd get rid of the enumeration type DataSupplierType as the classes TrendingResponse (and others) already provide a discrimination criteria.

(Also keep in mind that enums are full classes)

The perfect response to this would have you implement a basic type for your response, e.g:

interface Response {
    int getScore(); // implement in subclasses
    String getName(); // implement in subclasses
}

class TrendingResponse implements Response {}
class SkillResponse implements Response {}
class OtherResponse implements Response {}

but this is not strictly necessary.

At this point just a generic wrapper class would be enough (no need to have a base interface and extend it for each type of response):

class DataSupplierResponse<T extends Response>  {
    private T t;

    public DataSupplierResponse(final T t) {
        this.t = t;
    }
    public T getSupplierResponse() {
        return this.t;
    }
}

This would allow you to call:

Optional<DataSupplierResponse<?>> first = data.responses
            .stream()
            .filter(response -> TrendingResponse.class.isAssignableFrom(response.getClass()))
            .findFirst();

first.ifPresent( o -> o.getSupplierResponse().getScore() );

or simply

Optional<?> first = data.responses
            .stream()
            .filter(response -> TrendingResponse.class.isAssignableFrom(response.getClass()))
            .map(r -> r::getSupplierResponse)
            .findFirst();

Even without the base interface Response (which is useful only for defining common behavior in your responses), your class DataSupplierResponse<T> wouldn't need the enumeration type.

payloc91
  • 3,724
  • 1
  • 17
  • 45