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

Sampling to collect data

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

Problem

I am working on some sampling to collect data for my research on SOLID Principles. One of my sample consists proceeding code snippet:

public abstract class Notify
{
    public abstract void NotifyClient();
}

public class OnPremisesClient : Notify
{
    public override void NotifyClient()
    {
        Console.WriteLine("You're getting these notifications because you opted....");
    }
}

public class CloudClient : Notify
{
    public override void NotifyClient()
    {
        Console.WriteLine("You're getting these notifications because you opted....");

        if (IsOnPremisesToo)
            NotifyClientAsOnPremisesClient();
    }

    public void NotifyClientAsOnPremisesClient()
    {
        Console.WriteLine("Awesome! You are also using On premises services...");
    }

    public bool IsOnPremisesToo { get; set; }
}


Calling class is:

public  class Program
    {
         public static void Main(string[] args)
          {
              var premisesClient = new OnPremisesClient();
              var cloudClient = new CloudClient();

              ProcessNotifications(new List { premisesClient, cloudClient });
          }

          private static void ProcessNotifications(List list)
          {
               HandleItems(list);
          }

          static void HandleItems(IEnumerable notifications)
          {
              foreach (var notification in notifications)
              {
                 if (notification is CloudClient)
                 {
                     var cloudClient = notification as CloudClient;
                     cloudClient.IsOnPremisesToo = true;
                 }
                 notification.NotifyClient();
              }
          }
    }


In the preceding code snippet, I am trying to notify the client as per the type of Notify client could be a OnPremisesClient or CloudClient.

This code-snippet looks neat and clean, but I would like to discuss which SOLID principle it violates. After going through few

Solution

The code in example is violating following principles:

  • Single Responsibility Principle(SRP):


The responsibility of Notifying is spread thin across Clients and the classes are not clear with their intentions and it is also not reflecting through the behaviours. Now, it is not clear whether the client is also supposed to do other jobs.

  • Liscov Substitute Principle(LSP):


'Is-A' Relationship should be replaced with 'Is-Substitue-for' relationship. notification is CloudClient is violating LSP.

  • Dependency Inversion Principle(DIP): is violated since program class is dependent on client implementations and Notify class.



IMO, code can be restructured as follows. Please note, it can be also improved with better DI Implementation. By judging the code listing, I am not clear what is intended output of this program. Whether it is sending two different notifications or three notifications. Still,trying to provide the answer as per my understanding and assumptions below. Please let me know in comments the intent of the program so that I can modify the program as per the intent.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace CodeSmellQuestion
{
    public class Notification
    {
        private readonly List _providerList;
        public Notification(List providerList)
        {
            _providerList = providerList;
        }

        public void SendAll()
        {
            foreach (var notificationProvider in _providerList)
            {
                notificationProvider.Notify();
            }
        }
    }

    public interface INotify
    {
        void Notify();
    }

    public class OnPremiseNotifier : INotify
    {
        public void Notify()
        {
            Console.WriteLine("You're getting these notifications because you opted for OnPremise Notifications....");
        }

    }

    public class CloudNotifier : INotify
    {
        public void Notify()
        {
            Console.WriteLine("You're getting these notifications because you opted for Cloud Notifications....");
         }
    }
}

Code Snippets

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace CodeSmellQuestion
{
    public class Notification
    {
        private readonly List<INotify> _providerList;
        public Notification(List<INotify> providerList)
        {
            _providerList = providerList;
        }

        public void SendAll()
        {
            foreach (var notificationProvider in _providerList)
            {
                notificationProvider.Notify();
            }
        }
    }

    public interface INotify
    {
        void Notify();
    }

    public class OnPremiseNotifier : INotify
    {
        public void Notify()
        {
            Console.WriteLine("You're getting these notifications because you opted for OnPremise Notifications....");
        }

    }

    public class CloudNotifier : INotify
    {
        public void Notify()
        {
            Console.WriteLine("You're getting these notifications because you opted for Cloud Notifications....");
         }
    }
}

Context

StackExchange Code Review Q#138643, answer score: 3

Revisions (0)

No revisions yet.