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

Are redundancy and reflection my only options here?

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

Problem

I have a function - 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:

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.