patternjavaspringMinor
REST service to list new, continuing, and terminated subscribers during some period
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:
Example JSON output:
The way I implemented it is:
Controller
Response
Is it the right way of implementing it? What are the pros (if any) and cons?
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
-
The current solution modifies
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.
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.