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

How can the below code can be refactored with design pattern?

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

Problem

In one of our project with 3 tier architecture (tightly coupled), there's a bad code smell and it doesn't follow the DRY principle. I want to refactor it with design possible design pattern. I don't want to write this project from scratch.

Here's one of the class in BLL:

namespace BAL
{
   public class NotificationSettings
   {
    public readonly bool SignUPFBPost;
    public readonly bool SignUPEmails;
    public readonly bool SignUPPushNotification;
    public readonly bool SignUPFCActivity;
    public readonly bool SignUPBubbleNotification;
    public readonly bool SignUPGlobeNotification;

    public NotificationSettings()
    {
        NotificationSettingsBAL objNotificationSettings = new NotificationSettingsBAL();
        DataTable dtblNotifList = objNotificationSettings.GetNotificationList();
        SignUPFBPost = (Convert.ToInt16(dtblNotifList.Rows[0]["FBWallPost"]) == 1) ? true : false;
        SignUPEmails = (Convert.ToInt16(dtblNotifList.Rows[0]["Emails"]) == 1) ? true : false;
        SignUPPushNotification = (Convert.ToInt16(dtblNotifList.Rows[0]["PushNotification"]) == 1) ? true : false;
        SignUPFCActivity = (Convert.ToInt16(dtblNotifList.Rows[0]["FCActivity"]) == 1) ? true : false;
        SignUPBubbleNotification = (Convert.ToInt16(dtblNotifList.Rows[0]["BubbleNotification"]) == 1) ? true : false;
        SignUPGlobeNotification = (Convert.ToInt16(dtblNotifList.Rows[0]["GlobNotification"]) == 1) ? true : false;
    }
   }
}


We're getting various user related settings db from database and checking at different places as per the need and displaying messages or sending push notification.

```
if (objSqlResult.IsSuccess)
{
FavorBAL.Favor objFavorStatus = (FavorBAL.Favor)objFavorBAL.Status;

switch (objFavorStatus)
{
case FavorBAL.Favor.Initiated:
break;
case FavorBAL.Favor.Accepted:
strTitle = "Accept";
strMessage = MsgSuccess.GetMsg(Successes.FBPostFavorAccept).Replace("{

Solution

I would look at replacing the BLL code with the following. With the main focus being replacing the Convert.ToInt16 repetition with a method. The other area of concern is the inability to mock your BAL code. To overcome this I have written a quick and dirty interface which allows better unit testing.

namespace BAL
{
    // interfaces allow the class ot be easily mocked
    internal interface ICustomDataColumnTryParse
    {
        bool TryParseBool(string columnName);
    }

    // interfaces allow the class ot be easily mocked
    internal interface INotificationSettingsDomain
    {
        DataTable GetNotificationList();
    }

    // sample implementation of the interface
    internal class NotificationSettingsBAL : INotificationSettingsDomain
    {
        public DataTable GetNotificationList()
        {
            // do whatever you need to return information from your data repository
            throw new System.NotImplementedException();
        }
    }

    internal class NotificationSettings : ICustomDataColumnTryParse
    {
        // properties are more widely accepted for consumers of this class over
        // traditional fields / variables, but it really depends on 'what' you want
        // to use these values for?
        internal bool SignUPFBPost { get; private set; }
        internal bool SignUPEmails { get; private set; }
        internal bool SignUPPushNotification { get; private set; }
        internal bool SignUPFCActivity { get; private set; }
        internal bool SignUPBubbleNotification { get; private set; }
        internal bool SignUPGlobeNotification { get; private set; }

        private DataTable m_notificationListDataTable;

        internal NotificationSettings()
            : this(new NotificationSettingsBAL())
        { }

        // using an interface allows mocking of the 'NotificationSettingsBAL' object
        public NotificationSettings(INotificationSettingsDomain notificationSettingsDomain)
        {
            // prefer suffixes over prefixes for naming conventions
            m_notificationListDataTable = notificationSettingsDomain.GetNotificationList();

            SignUPFBPost = TryParseBool("FBWallPost");
            SignUPEmails = TryParseBool("Emails");
            SignUPPushNotification = TryParseBool("PushNotification");
            SignUPFCActivity = TryParseBool("FCActivity");
            SignUPBubbleNotification = TryParseBool("BubbleNotification");
            SignUPGlobeNotification = TryParseBool("GlobNotification");
        }

        // this can be a method on this class, extension method (my preference) or a static method
        // i have opted for an internal class method coded against an interface so the data table 
        // isn't passed around. 
        public bool TryParseBool(string columnName)
        {
            // this can allow for additional logic if we wanted to get a different row index
            var row = m_notificationListDataTable.Rows[0];
            var value = row[columnName];

            if (value == null)
            {
                return false;
            }

            int valueAsInt = 0;
            int.TryParse(value.ToString(), out valueAsInt);
            return valueAsInt == 1;
        }
    }
}


As for replacing the switch statement you might be over-using an architectural pattern. I would only choose something like a strategy pattern if the business logic is likely to change or that additional switch statements will be added / removed with a high frequency in the future.

Code Snippets

namespace BAL
{
    // interfaces allow the class ot be easily mocked
    internal interface ICustomDataColumnTryParse
    {
        bool TryParseBool(string columnName);
    }

    // interfaces allow the class ot be easily mocked
    internal interface INotificationSettingsDomain
    {
        DataTable GetNotificationList();
    }

    // sample implementation of the interface
    internal class NotificationSettingsBAL : INotificationSettingsDomain
    {
        public DataTable GetNotificationList()
        {
            // do whatever you need to return information from your data repository
            throw new System.NotImplementedException();
        }
    }

    internal class NotificationSettings : ICustomDataColumnTryParse
    {
        // properties are more widely accepted for consumers of this class over
        // traditional fields / variables, but it really depends on 'what' you want
        // to use these values for?
        internal bool SignUPFBPost { get; private set; }
        internal bool SignUPEmails { get; private set; }
        internal bool SignUPPushNotification { get; private set; }
        internal bool SignUPFCActivity { get; private set; }
        internal bool SignUPBubbleNotification { get; private set; }
        internal bool SignUPGlobeNotification { get; private set; }

        private DataTable m_notificationListDataTable;

        internal NotificationSettings()
            : this(new NotificationSettingsBAL())
        { }

        // using an interface allows mocking of the 'NotificationSettingsBAL' object
        public NotificationSettings(INotificationSettingsDomain notificationSettingsDomain)
        {
            // prefer suffixes over prefixes for naming conventions
            m_notificationListDataTable = notificationSettingsDomain.GetNotificationList();

            SignUPFBPost = TryParseBool("FBWallPost");
            SignUPEmails = TryParseBool("Emails");
            SignUPPushNotification = TryParseBool("PushNotification");
            SignUPFCActivity = TryParseBool("FCActivity");
            SignUPBubbleNotification = TryParseBool("BubbleNotification");
            SignUPGlobeNotification = TryParseBool("GlobNotification");
        }

        // this can be a method on this class, extension method (my preference) or a static method
        // i have opted for an internal class method coded against an interface so the data table 
        // isn't passed around. 
        public bool TryParseBool(string columnName)
        {
            // this can allow for additional logic if we wanted to get a different row index
            var row = m_notificationListDataTable.Rows[0];
            var value = row[columnName];

            if (value == null)
            {
                return false;
            }

            int valueAsInt = 0;
            int.TryParse(value.ToString(), out valueAsInt);
            return valueAsInt == 1;
        }
    }
}

Context

StackExchange Code Review Q#7306, answer score: 5

Revisions (0)

No revisions yet.