patternjavaMinor
Concatenating multiple feedback messages
Viewed 0 times
concatenatingfeedbackmessagesmultiple
Problem
The goal is to build a
```
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(
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
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...
Why does the
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 justfeedbackMessage.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.