0
\$\begingroup\$

I am implementing observer pattern(refering codeproject) for stock quote update. I have created an interface for Subject and Observer. I have created instances for Subject and Observer and also the functionality to register, notify and update. Kindly let me know the approach is correct

public  interface IObserver
    {
        void update(double value);
    }
}

//subject

public interface ISubject
    {
         void Register(IObserver o);
         void UnRegister(IObserver o);
         void notify();

    }

//Observer instance

public class InfosysStock : IObserver
    {
        private ISubject _subject;
        private double _latestvalue = 0;
        public InfosysStock(ISubject subject)
        {
            _subject = subject;
            _subject.Register(this);
        }



        public void update(double value)
        {
            _latestvalue = value;
            display();
        }

        private void display()
        {
            Console.WriteLine("The latest value is : " + _latestvalue);
        }


        public void unsubscribe()
        {
            _subject.UnRegister(this);

        }
    }

//Subject Instance

public class StockMarket : ISubject
    {

        List<IObserver> observers = new List<IObserver>();
        public int _stockvalue = 0;

        public void setValue(int v)
        {
            _stockvalue = v;
            notify();
        }

        public void notify()
        {
            foreach (var observer in observers)
            {
                observer.update(_stockvalue);
            }
        }

        public void Register(IObserver o)
        {
            observers.Add(o);
        }

        public void UnRegister(IObserver o)
        {
            int idx = observers.IndexOf(o);

            observers.RemoveAt(idx);

        }



    }

Finally the main class

StockMarket stockmarket = new StockMarket();
InfosysStock infy = new InfosysStock(stockmarket);
stockmarket.setValue(15);
stockmarket.setValue(21);

            infy.unsubscribe();
            Console.ReadLine();
\$\endgroup\$
4
  • \$\begingroup\$ This is not how it is supposed to be used. setValue should automatiacally notify listeners. Having to call notify manually is pointless and makes the whole pattern unnecessary. \$\endgroup\$
    – t3chb0t
    Commented Oct 4, 2018 at 6:15
  • \$\begingroup\$ @t3chb0t I have modified the code. Does it look fine now? any other modifications to be made? \$\endgroup\$
    – krishna_v
    Commented Oct 4, 2018 at 10:10
  • \$\begingroup\$ Is this just to learn? why not use the built in interfaces and reactive extensions? \$\endgroup\$ Commented Oct 5, 2018 at 2:23
  • \$\begingroup\$ I am implementing it newly and would like to get the feedback if the approach is right? \$\endgroup\$
    – krishna_v
    Commented Oct 5, 2018 at 14:38

1 Answer 1

3
\$\begingroup\$

1. If you are implementing this pattern for learning, then it is OK.

Otherwise, you can use built-in Observer Pattern of C#.

2. Think about readable:

    public void UnRegister(IObserver o)
    {
         int idx = observers.IndexOf(o);

         observers.RemoveAt(idx);

     }


 }

There are some redundant empty lines in your code. Inaddition, "o" is not a good variable name. I think you can change it to "observer".

3.You don't need to retrieve index of an object and then you remove it via the index. It will reduce the performance:

      public void UnRegister(IObserver o)
      {
          int idx = observers.IndexOf(o);

          observers.RemoveAt(idx);

       }

You can use Remove() method to remove an object from the list directly:

    public void UnRegister(IObserver o)
    {
        observers.Remove(o);
    }
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.