Generating a random numbers and using INotifyPropertyChanged to Updating UI

Question

I'm new to c# and currently working on a program. it has a simple UI, there are two buttons (one is called on and other one is off) and also a textbox to show some result. basically what I want to do is that if the user click "on" button, on a different class than the windows form random numbers will be generated every one second using a method. and by Implementing INotifyPropertyChanged I want to let the Textbox know that the value was updated so textbox keeps getting updated with our new random number. and once user click "off" button I want to stop generating random number.

My windows Form

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Windows.Forms;

namespace SWE_Assignment
{
    public partial class Monitor : Form
    {
        Patient newPatient = Patient.Instance;
        public static bool pulseRateOn = false;
        public Monitor()
        {
            InitializeComponent();

            newPatient.PropertyChanged += _PulseRate_PropertyChanged;
        }

        private void Save_Click(object sender, EventArgs e)
        {

           // newPatient.PL(newPatient.startRnd = true;);
        }

        void _PulseRate_PropertyChanged(object sender, PropertyChangedEventArgs e)
        {
            if (e.PropertyName == "PulseRate")
            {
                HeartBeat.Text = newPatient.PulseRate.ToString();
            }
        }



        private void Stop_Click(object sender, EventArgs e)
        {
            newPatient.startRnd = false;
        }
    }
}

My Patient Class

namespace SWE_Assignment
{
    class Patient : INotifyPropertyChanged
    {

        private int _pulseRate;
        public bool startRnd = false;
        Random rnd = new Random();
        public int PulseRate
        {
            get { return _pulseRate; }
            set
            {
                _pulseRate = value;
                OnPropertyChanged("PL");
            }
        }


        public event PropertyChangedEventHandler PropertyChanged;
        private void OnPropertyChanged(string properyName)
        {
            PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(properyName));
        }

        private static Patient _instance = null;
        private static readonly object _padlock = new object();
        public static Patient Instance
        {
            get
            {
                lock (_padlock)
                {
                    if (_instance == null)
                    {
                        _instance = new Patient();
                    }
                    return _instance;
                }
            }
        }




        public void PL(bool srt)
        {
            Timer timer = new Timer();
            timer.AutoReset = true;
            timer.Interval = 1000;


            if (startRnd == true)
            {
                timer.Elapsed += PLS;
                timer.Enabled = true;
                timer.Start();
            } else
            {
                timer.Enabled = false;
                timer.Stop();
            }
        }

        private void PLS(object sender, ElapsedEventArgs e)
        {

            PulseRate = rnd.Next(100, 150);
            Console.WriteLine(PulseRate);

        }
    }
}

Also my I am using singleton pattern for my patient because I want to only have on instance of patient so that I can have access to the same random number on another class (called Alarm) to check if its bigger or smaller than a certain numbers. I do realise that is wrong with my "Stop" button as it only calls the method again and it doesn't stop method from running. I appreciate if anyone can help.

Answer 1

there are a few things which need to be changed.

OnPropertyChanged("PL"); - the name of property is PulseRate. "PulseRate" is what you check in event handler. make it OnPropertyChanged("PulseRate");

Timer timer = new Timer(); - you should create only one instance of Timer per Patient. better do it in constructor. otherwise you will have multiple running copies of Timer

managing Timer can be done via Enabled property: Calling the Start() method is the same as setting Enabled to true. Likewise, calling the Stop() method is the same as setting Enabled to false.

private void Stop_Click(object sender, EventArgs e)
{
    // newPatient.startRnd = false;
    newPatient.PL(false);
}

class Patient : INotifyPropertyChanged
{
    Timer timer;
    private Patient()
    {
        timer = new Timer();
        timer.AutoReset = true;
        timer.Interval = 1000;
        timer.Elapsed += PLS;
    }

    Random rnd = new Random();

    private int _pulseRate;
    public int PulseRate
    {
        get { return _pulseRate; }
        set
        {
            _pulseRate = value;
            OnPropertyChanged("PulseRate");
        }
    }

    public void PL(bool srt)
    {
        timer.Enabled = srt;
    }

    private void PLS(object sender, ElapsedEventArgs e)
    {
        PulseRate = rnd.Next(100, 150);
        Console.WriteLine(PulseRate);
    }

    public event PropertyChangedEventHandler PropertyChanged;
    private void OnPropertyChanged(string properyName)
    {
        PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(properyName));
    }

    private static Patient _instance = null;
    private static readonly object _padlock = new object();
    public static Patient Instance
    {
        get
        {
            lock (_padlock)
            {
                if (_instance == null)
                {
                    _instance = new Patient();
                }
                return _instance;
            }
        }
    }
}