HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaspringMinor

REST service to list new, continuing, and terminated subscribers during some period

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
restnewperiodduringcontinuingservicesometerminatedandlist

Problem

I have a use case which says:


Your REST service will consume a JSON object that contains a list of
emails for subscribers at the start of the subscription period, and a
list of subscribers at the end of the subscription period. Your
service must return a list of subscribers who are new, subscribers who
stayed, and subscribers who unsubscribed.


Example JSON input:

{
    "start" : [
        "test1@test.com",
        "test2@test.com"
    ],
    "end" : [
        "test1@test.com",
        "test3@test.com"
    ]
}




Example JSON output:

{
    "new" : [
        "test3@test.com"
    ],
    "stayed" : [
        "test1@test.com"
    ],
    "unsubscribed" : [
        "test2@test.com"
    ]
}


The way I implemented it is:

Controller

@POST
@Timed
@Consumes(MediaType.APPLICATION_JSON)
public SubscriptionResponse subscriptionDetail(SubscriptionRequest request) {
    return new SubscriptionResponse(request);
}


Response

public SubscriptionResponse (SubscriptionRequest request) {
        //Get the common subscriptions
        List common = new ArrayList();
        common.addAll(request.getEnd());
        common.retainAll(request.getStart());
        //Filter out unsubscribed
        List unsubscribed = request.getStart();
        unsubscribed.removeAll(common);
        //Get the new subscription
        List newSub = request.getEnd();
        newSub.removeAll(common);
        unsubscribed.removeAll(common);

        this.newSub = newSub;
        this.stayed = common;
        this.unsubscribed = unsubscribed;
    }


Is it the right way of implementing it? What are the pros (if any) and cons?

Solution

This is one way of implementing it.
Whether it's right or not depends on many conditions,
and we could only speculate.
Here are some example problems anyway,
that could be serious issues:

-
The current solution is inefficient for large lists, because for example, for list1.retainAll(list2) the worst case complexity is \$O(N M)\$ where \$N\$ and \$M\$ are the sizes of the two lists. (For more details, see the source code of ArrayList.retainAll. You could make that \$O(N)\$ by converting list2 to a Set.

-
The current solution modifies request.getStart() and request.getEnd(). Maybe that's not ok, then the current solution should be adjusted to work with copies instead.

On the other hand,
a good thing about the current solution is that it's short and idiomatic,
and it doesn't waste memory (at the expense of CPU).

Lastly,
a minor issue is that the comments are pointless,
they only state the obvious.
The code would be easier to read if you made those lines empty.

Context

StackExchange Code Review Q#91514, answer score: 2

Revisions (0)

No revisions yet.