This class looks ugly as hell to me. I feel like there is a better way but can't really think for a good one. Too many instanceofs and else ifs. What OOP practices or design patterns would you suggest to change and improve the code?
Any tips, advises and recommended resources are welcome!
What does the code do: Basically, public method accepts event object and every event object contains user/player object in some form. The reason we are getting user object from the event is to check whether that particular user will receive notification about the event. Which of course depends on user's notification settings which is inside user object.
I've added ENUM class, maybe there is a way to modify it for a better code quality.
@Service
public class NotificationSettingsCheckServiceImpl implements NotificationSettingsCheckService {
@Override
public Boolean checkIfEventCanPass(ApplicationEvent event) {
if (SYSTEM_EVENTS.getListOfClasses().contains(event.getClass())) {
return true;
}
Boolean eventPasses = false;
LotteryUser user;
user = event instanceof NotificationApplicationEvent ? getUserBasedOnNotificationEvent(event) : getUserBasedOnEmailEvent(event);
if (CAMPAIGN_EVENTS.getListOfClasses().contains(event.getClass()) && user.getNotificationSettings().getCampaignEvents()) {
eventPasses = true;
} else if (DRAW_RESULT_EVENTS.getListOfClasses().contains(event.getClass()) && user.getNotificationSettings().getDrawResultEvents()) {
eventPasses = true;
} else if (TRANSACTION_EVENTS.getListOfClasses().contains(event.getClass()) && user.getNotificationSettings().getTransactionEvents()) {
eventPasses = true;
} else if (USER_WON_EVENTS.getListOfClasses().contains(event.getClass()) && user.getNotificationSettings().getTransactionEvents()) {
eventPasses = true;
}
return eventPasses;
}
private LotteryUser getUserBasedOnEmailEvent(EventObject event) {
LotteryUser user = null;
if (event instanceof UserThanksEvent) {
user = ((UserThanksEvent) event).getUser();
} else if (event instanceof UserOrderCanceledEvent) {
user = ((UserOrderCanceledEvent) event).getUser();
} else if (event instanceof DrawResultEvent) {
user = ((DrawResultEvent) event).getPlayer();
} else if (event instanceof UserWinCongratulationEvent) {
user = ((UserWinCongratulationEvent) event).getPlayer();
} else if (event instanceof UserAddedToCampaignEvent) {
user = ((UserAddedToCampaignEvent) event).getLotteryUser();
}
return user;
}
private LotteryUser getUserBasedOnNotificationEvent(EventObject event) {
LotteryUser user = null;
if (event instanceof UserReceivedBonusMoneyEvent) {
user = ((UserReceivedBonusMoneyEvent) event).getLotteryUser();
} else if (event instanceof UserReceivedBonusInNonDepositCampaignEvent) {
user = ((UserReceivedBonusInNonDepositCampaignEvent) event).getLotteryUser();
} else if (event instanceof UserReceivedBonusInDepositCampaignEvent) {
user = ((UserReceivedBonusInDepositCampaignEvent) event).getLotteryUser();
} else if (event instanceof UserTakesPartInDepositCampaignEvent) {
user = ((UserTakesPartInDepositCampaignEvent) event).getUser();
} else if (event instanceof UserTakesPartInNonDepositCampaignEvent) {
user = ((UserTakesPartInNonDepositCampaignEvent) event).getUser();
} else if (event instanceof DrawResultNotificationEvent) {
user = ((DrawResultNotificationEvent) event).getPlayer();
}
return user;
}
}
Here is the ENUM class I'm using.
public enum NotificationSettingsType {
SYSTEM_EVENTS(Arrays.asList(OnRegistrationCompleteEvent.class,
ResetPasswordEvent.class,
ManagerRegistrationEvent.class,
DrawResultNotFoundEvent.class,
CurrencyUpdatedEvent.class,
WalletLockedEvent.class,
UserWonBigPrizeEvent.class)),
CAMPAIGN_EVENTS(Arrays.asList(UserReceivedBonusInDepositCampaignEvent.class,
UserReceivedBonusInNonDepositCampaignEvent.class,
UserReceivedBonusMoneyEvent.class,
UserTakesPartInDepositCampaignEvent.class,
UserTakesPartInNonDepositCampaignEvent.class,
UserAddedToCampaignEvent.class)),
DRAW_RESULT_EVENTS(Arrays.asList(DrawResultNotificationEvent.class,
DrawResultEvent.class)),
TRANSACTION_EVENTS(Arrays.asList(UserThanksEvent.class,
UserOrderCanceledEvent.class)),
USER_WON_EVENTS(Collections.singletonList(UserWinCongratulationEvent.class));
private List<Class> listOfClasses;
NotificationSettingsType(List<Class> listOfClasses) {
this.listOfClasses = listOfClasses;
}
public List<Class> getListOfClasses() {
return listOfClasses;
}
}
Here are the Event Object example and the class that it extends. I have several of these depending on the event they notify. This is just an example. Using Lombok plugin for Getter/Setter annotations if that confuses anyone.
@Getter
@Setter
public class UserReceivedBonusMoneyEvent extends NotificationApplicationEvent {
private final LotteryUser lotteryUser;
private final BigDecimal bonusAmount;
private final String currency;
public UserReceivedBonusMoneyEvent( LotteryUser lotteryUser, BigDecimal bonusAmount, String currency) {
super(bonusAmount);
this.lotteryUser = lotteryUser;
this.bonusAmount = bonusAmount;
this.currency = currency;
}
@Override
public void accept(NotificationEventVisitor visitor) {
visitor.visit(this);
}
}
And here is the NotificationApplicationEvent, which extends Spring framework's Application Event.
public abstract class NotificationApplicationEvent extends ApplicationEvent {
/**
* Create a new ApplicationEvent.
*
* @param source the object on which the event initially occurred (never {@code null})
*/
public NotificationApplicationEvent(Object source) {
super(source);
}
public abstract void accept(NotificationEventVisitor visitor);
}
And This is NotificationEventListener
class where checkIfEventCanPass
method is placed.
@Component
@Slf4j
public class NotificationEventListener implements ApplicationListener<NotificationApplicationEvent> {
@Autowired
private NotificationEventVisitor notificationEventVisitor;
@Autowired
private NotificationSettingsCheckService notificationSettingsCheckService;
@Override
public void onApplicationEvent(NotificationApplicationEvent event) {
if (notificationSettingsCheckService.checkIfEventCanPass(event)) {
event.accept(notificationEventVisitor);
}
}
}
EventObject
class and all/most of its decendant classes. \$\endgroup\$instanceof
tests... \$\endgroup\$EmailApplicationEvent
which is totally same likeNotificationApplicationEvent
. \$\endgroup\$