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

Concatenating multiple feedback messages

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

Problem

The goal is to build a String from a given set where Feedback is an interface and DeliveryFeedback, and ProductFeedbackare its implementing classes.

```
private void formatOrderFeedbackZendeskBody(final ImmutableSet feedback) {
StringBuilder feedbackMessage = new StringBuilder("The feedback: ");
feedback.forEach(
f -> {
if (f instanceof DeliveryFeedback) {
feedbackMessage
.append("\n\nType: ")
.append(FeedbackType.DELIVERY)
.append("\nReason(s): ")
.append(
((DeliveryFeedback) f)
.getReasons()
.stream()
.map(Reason::name)
.collect(joining("\n\t\t\t")));
if (((DeliveryFeedback) f).getComment().isPresent()) {
feedbackMessage
.append("\nComment: ")
.append(((DeliveryFeedback) f).getComment().get());
}
} else if (f instanceof ProductFeedback) {
feedbackMessage
.append("\n\nType: ")
.append(FeedbackType.PRODUCT)
.append("\nReason(s): ")
.append(
((ProductFeedback) f)
.getReasons()
.stream()
.map(

Solution

Polymorphism failure. Determine who is responsible for deciding what the output is supposed to look like and go from there. If the feedback is responsible, just implement and overload toString(). If the printer is responsible, find a way to get most of the detailed information into the interface.

if (f instanceof DeliveryFeedback) {
} else if (f instanceof ProductFeedback) {


This is just wrong.

Basically, what's the point of having the interface if you're just gonna side-step the interface and take a look at the implementing class?

For example...

if (((DeliveryFeedback) f).getComment().isPresent()) {
    feedbackMessage
            .append("\nComment: ")
            .append(((DeliveryFeedback) f).getComment().get());
}
if (((ProductFeedback) f).getComment().isPresent()) {
    feedbackMessage
            .append("\nComment: ")
            .append(((ProductFeedback) f).getComment().get());
}


Why does the Feedback interface not have a getComment method? Then you could just

feedbackMessage.append(f.getComment().map(c -> "\nComment: " + c).orElse(""));

Code Snippets

if (f instanceof DeliveryFeedback) {
} else if (f instanceof ProductFeedback) {
if (((DeliveryFeedback) f).getComment().isPresent()) {
    feedbackMessage
            .append("\nComment: ")
            .append(((DeliveryFeedback) f).getComment().get());
}
if (((ProductFeedback) f).getComment().isPresent()) {
    feedbackMessage
            .append("\nComment: ")
            .append(((ProductFeedback) f).getComment().get());
}
feedbackMessage.append(f.getComment().map(c -> "\nComment: " + c).orElse(""));

Context

StackExchange Code Review Q#161404, answer score: 6

Revisions (0)

No revisions yet.