Stack Code Review

Using delegates to communicate between forms and networkcommunicator class

Problem

I have multiple forms, each of which would like to send message through the network, using my NetworkCommunicator class’s SendMessage method.

The solution I described here works, however it is ugly, and bludgeons some of the main concepts of OOP paradigm. What I’m looking for (and I’m open to any suggestions) is basically to achieve the same goal but in a more elegant, and OOP conforming solution. (maybe by events?)

I’m using more than these forms, but for the example, these seemed to be enough.

//using statements
namespace examplespace
{
 public delegate void ImprovedSenderDelegate(string message);  

 public class LoginForm :Form
  {
   private  NetworkCommunicator ncom = new NetworkCommunicator();
   public Ncom
   {
    get { return ncom; }
   }

   private void Login()
   {
    MainForm mf = new MainForm();
    mf.Owner = this;
    //other things i do etc.
    this.Hide();
    mf.Show();
   }      
  }//endofLoginForm

 public class MainForm :Form
 {
  public ImprovedSenderDelegate ISD;

  private void Load ()
  {
   (this.Owner as LoginForm).NCom.SubscribeToSendMessage(this);
  }

  private void I_Want_To_send_A_Message(string message)
  {
   ISD(string);
  }
 }//endofMainForm

 public class NetworkCommunicator
  {
   public void SubscribeToSendMessage(object sender)
    {
     if (sender is MainForm)
     {

         (sender as MainForm).ISD += new ImprovedSenderDelegate(SendMessage);
     }
    }

   private void SendMessage(string message) 
    {/*magic*/}
  }/endofNetworkCommunicator
}

Solution

My suggestions:

If the NetworkCommunicator exists only once per application, you should create a singelton. Code sample:

    /// <summary>
    /// Provides a base class for singeltons.
    /// </summary>
    /// <typeparam name="T">The class that act as a singelton.</typeparam>
    public abstract class Singleton<T>
        where T : class
    {
        protected Singleton()
        {
        }

        /// <summary>
        /// Gets the singelton instance.
        /// </summary>
        public static T Instance
        {
            get { return Nested._instance; }
        }

        private sealed class Nested
        {
            /// <summary>
            /// Creates the nested instance class.
            /// </summary>
            /// 
            /// <remarks>
            /// Explicit static constructor to tell the compiler
            //  not to mark type as beforefieldinit.
            /// </remarks>
            static Nested()
            {
                ConstructorInfo constructor = typeof (T).GetConstructor(Type.EmptyTypes) ??
                                              typeof(T).GetConstructor(BindingFlags.NonPublic | BindingFlags.Instance, null, Type.EmptyTypes, null);

                if (constructor == null)
                    throw new InvalidOperationException("Class has no private nor public constructor");

                _instance = (T)constructor.Invoke(null);
            }

            internal static readonly T _instance;
        }
    }

If someone has any suggestion about my singelton implementation, let me know it..

The delegate makes the code more complex and violates the KISS OOP principle: Keep It Simple and Stupid. Make it as easy as possible first.

Exit mobile version