patterncsharpMinor
Are redundancy and reflection my only options here?
Viewed 0 times
reflectionhereareoptionsredundancyandonly
Problem
I have a function -
The code looks like this:
So the only purpose of this method is to determine which settings should be used. Every
I think this is a good use case for reflection, but want to ask if there's any other way to reduce redundancy here. I don't like reflection because it's not as straightforward to debug, and it's slow (right?).
UPDATE
CreateNotification:
User:
```
public class User
TriggerNotifications - that accepts a NotificationType. The Type determines which settings I should pull from the User. The code looks like this:
public void TriggerNotification(DbContext db, User user, NotificationType type, string content) {
switch (type) {
case NotificationType.Followed:
CreateNotification(db, user, user.Followed, content);
return;
case NotificationType.Unfollowed:
CreateNotification(db, user, user.Unfollowed, content);
return;
case NotificationType.Messaged:
CreateNotification(db, user, user.Messaged, content);
return;
default:
throw new InvalidDataContractException("Invalid Notification Type");
}
}So the only purpose of this method is to determine which settings should be used. Every
User has a property matching each of the enum NotificationTypes. I think this is a good use case for reflection, but want to ask if there's any other way to reduce redundancy here. I don't like reflection because it's not as straightforward to debug, and it's slow (right?).
UPDATE
CreateNotification:
private void CreateNotification(DbContext db, User user, NotificationSetting setting, string content) {
// send email, notification, or both
if (setting.Equals(NotificationSetting.None))
return;
var notification = new Notification() {
UserId = user.userId
Description = content,
Email = email,
};
if (setting.Equals(NotificationSetting.Email) || setting.Equals(NotificationSetting.Both)) {
// tell worker role to pick it up
notification.PendingEmail = true;
}
// signalR stuff here
db.Add(notification);
}User:
```
public class User
Solution
Some of the redundancy is accidental, and some essential to your design (not essential to problem though, if it were essential to problem reflection wouldn't help).
Accidental redundancy can be removed thus:
Note:
-
I changed the exception as
-
Now that this is out of the way,
What can you change in the design to remove redundancy?
Your user entity is not normalized. If you really were doing code-first design,
Then
Another redundancy is in
Then
becomes
Accidental redundancy can be removed thus:
private static NotificationSetting GetNotificationSetting(User user, NotificationType type)
{
switch (type) {
case NotificationType.Followed: return user.Followed;
case NotificationType.Unfollowed: return user.Unfollowed;
case NotificationType.Messaged: return user.Messaged;
default:
throw new InvalidEnumArgumentException("Invalid Notification Type");
}
}
public void TriggerNotification(object db, User user, NotificationType type, string content) {
CreateNotification(db, user, GetNotificationSetting(user,type), content);
}Note:
-
I changed the exception as
InvalidDataContractException seems to be serialization related.-
GetNotificationSetting should be moved to User (or made an extension method of that class).Now that this is out of the way,
What can you change in the design to remove redundancy?
Your user entity is not normalized. If you really were doing code-first design,
User would look like this instead:class User: Settings
{
}
public abstract class Settings {
public IDictionary NotificationSettings { get; set; }
}Then
TriggerNotification becomes just:public void TriggerNotification(object db, User user, NotificationType type, string content) {
CreateNotification(db, user, user.NotificationSettings[type], content);
}Another redundancy is in
NotificationSetting. It clearly is a bitmask.[Flags]
public enum NotificationSetting : uint
{
Navbar = 1,
Email = 2,
// can add more like so
Telegram = 4,
PersonalVisit = 8,
}Then
if (setting.Equals(NotificationSetting.Email) || setting.Equals(NotificationSetting.Both))becomes
if (setting.HasFlag(NotificationSetting.Email))Code Snippets
private static NotificationSetting GetNotificationSetting(User user, NotificationType type)
{
switch (type) {
case NotificationType.Followed: return user.Followed;
case NotificationType.Unfollowed: return user.Unfollowed;
case NotificationType.Messaged: return user.Messaged;
default:
throw new InvalidEnumArgumentException("Invalid Notification Type");
}
}
public void TriggerNotification(object db, User user, NotificationType type, string content) {
CreateNotification(db, user, GetNotificationSetting(user,type), content);
}class User: Settings
{
}
public abstract class Settings {
public IDictionary<NotificationType, NotificationSetting> NotificationSettings { get; set; }
}public void TriggerNotification(object db, User user, NotificationType type, string content) {
CreateNotification(db, user, user.NotificationSettings[type], content);
}[Flags]
public enum NotificationSetting : uint
{
Navbar = 1,
Email = 2,
// can add more like so
Telegram = 4,
PersonalVisit = 8,
}if (setting.Equals(NotificationSetting.Email) || setting.Equals(NotificationSetting.Both))Context
StackExchange Code Review Q#37809, answer score: 5
Revisions (0)
No revisions yet.