views:

80

answers:

2

I am using .Net 3.5 for now.

Right now I am using a using trick to disable and enable events around certain sections of code. The user can change either days, hours, minutes or total minutes, and that should not cause an infinite cascade of events (e.g. minutes changing total, total changing minutes, etc.) While the code does what I want, there might be a better / more straight-forward way. Do you know of any?

For brawny points:

This control will be used by multiple teams - I do not want to make it embarrassing. I suspect that I do not need to reinvent the wheel when defining hours in a day, days in week, etc. Some other standard .Net library out there must have it. Any other remarks regarding code? This using (EventHacker.DisableEvents(this)) business - that must be a common pattern in .Net ... changing the setting temporarily. What is the name of it? I'd like to be able to refer to it in a comment and also read up more on current implementations. In the general case not only a handle to the thing being changed needs to be remembered, but also the previous state (in this case previous state does not matter - events are turned on and off unconditionally). Then there is also a possibility of multi-threaded hacking. One could also utilize generics to make the code arguably cleaner. Figuring all this out can lead to a multi-page blog post. I'd be happy to hear some of the answers.

P.S. Does it seem like I suffer from obsessive compulsive disorder? Some people like to get things finished and move on; I like to keep them open ... there is always a better way.

// Corresponding Designer class is omitted.
using System;
using System.Windows.Forms;

namespace XYZ // Real name masked
{
    interface IEventHackable
    {
        void EnableEvents();
        void DisableEvents();
    }

    public partial class PollingIntervalGroupBox : GroupBox, IEventHackable
    {
        private const int DAYS_IN_WEEK      = 7;
        private const int MINUTES_IN_HOUR   = 60;
        private const int HOURS_IN_DAY      = 24;
        private const int MINUTES_IN_DAY    = MINUTES_IN_HOUR * HOURS_IN_DAY;
        private const int MAX_TOTAL_DAYS    = 100;

        private static readonly decimal MIN_TOTAL_NUM_MINUTES = 1; // Anything faster than once per minute can bog down our servers.
        private static readonly decimal MAX_TOTAL_NUM_MINUTES = (MAX_TOTAL_DAYS * MINUTES_IN_DAY) - 1; // 99 days should be plenty.
        // The value above was chosen so to not cause an overflow exception.
        // Watch out for it - numericUpDownControls each have a MaximumValue setting.

        public PollingIntervalGroupBox()
        {
            InitializeComponent();

            InitializeComponentCustom();
        }

        private void InitializeComponentCustom()
        {
            this.m_upDownDays.Maximum           = MAX_TOTAL_DAYS    - 1;
            this.m_upDownHours.Maximum          = HOURS_IN_DAY      - 1;
            this.m_upDownMinutes.Maximum        = MINUTES_IN_HOUR   - 1;
            this.m_upDownTotalMinutes.Maximum   = MAX_TOTAL_NUM_MINUTES;
            this.m_upDownTotalMinutes.Minimum   = MIN_TOTAL_NUM_MINUTES;
        }

        private void m_upDownTotalMinutes_ValueChanged(object sender, EventArgs e)
        {
            setTotalMinutes(this.m_upDownTotalMinutes.Value);
        }

        private void m_upDownDays_ValueChanged(object sender, EventArgs e)
        {
            updateTotalMinutes();
        }

        private void m_upDownHours_ValueChanged(object sender, EventArgs e)
        {
            updateTotalMinutes();
        }

        private void m_upDownMinutes_ValueChanged(object sender, EventArgs e)
        {
            updateTotalMinutes();
        }

        private void updateTotalMinutes()
        {
            this.setTotalMinutes(
                MINUTES_IN_DAY * m_upDownDays.Value + 
                MINUTES_IN_HOUR * m_upDownHours.Value + 
                m_upDownMinutes.Value);
        }

        public decimal TotalMinutes
        {
            get
            {
                return m_upDownTotalMinutes.Value;
            }
            set
            {
                m_upDownTotalMinutes.Value = value;
            }
        }

        public decimal TotalHours
        {
            set
            {
                setTotalMinutes(value * MINUTES_IN_HOUR);
            }
        }

        public decimal TotalDays
        {
            set
            {
                setTotalMinutes(value * MINUTES_IN_DAY);
            }
        }

        public decimal TotalWeeks
        {
            set
            {
                setTotalMinutes(value * DAYS_IN_WEEK * MINUTES_IN_DAY);
            }
        }

        private void setTotalMinutes(decimal nTotalMinutes)
        {
            if (nTotalMinutes < MIN_TOTAL_NUM_MINUTES)
            {
                setTotalMinutes(MIN_TOTAL_NUM_MINUTES);
                return; // Must be carefull with recursion.
            }
            if (nTotalMinutes > MAX_TOTAL_NUM_MINUTES)
            {
                setTotalMinutes(MAX_TOTAL_NUM_MINUTES);
                return; // Must be carefull with recursion.
            }
            using (EventHacker.DisableEvents(this))
            {
                // First set the total minutes
                this.m_upDownTotalMinutes.Value = nTotalMinutes;

                // Then set the rest
                this.m_upDownDays.Value = (int)(nTotalMinutes / MINUTES_IN_DAY);
                nTotalMinutes = nTotalMinutes % MINUTES_IN_DAY; // variable reuse.
                this.m_upDownHours.Value = (int)(nTotalMinutes / MINUTES_IN_HOUR);
                nTotalMinutes = nTotalMinutes % MINUTES_IN_HOUR;
                this.m_upDownMinutes.Value = nTotalMinutes;
            }
        }

        // Event magic
        public void EnableEvents()
        {
            this.m_upDownTotalMinutes.ValueChanged += this.m_upDownTotalMinutes_ValueChanged;
            this.m_upDownDays.ValueChanged += this.m_upDownDays_ValueChanged;
            this.m_upDownHours.ValueChanged += this.m_upDownHours_ValueChanged;
            this.m_upDownMinutes.ValueChanged += this.m_upDownMinutes_ValueChanged;
        }

        public void DisableEvents()
        {
            this.m_upDownTotalMinutes.ValueChanged -= this.m_upDownTotalMinutes_ValueChanged;
            this.m_upDownDays.ValueChanged -= this.m_upDownDays_ValueChanged;
            this.m_upDownHours.ValueChanged -= this.m_upDownHours_ValueChanged;
            this.m_upDownMinutes.ValueChanged -= this.m_upDownMinutes_ValueChanged;
        }

        // We give as little info as possible to the 'hacker'.
        private sealed class EventHacker : IDisposable
        {
            IEventHackable _hackableHandle;

            public static IDisposable DisableEvents(IEventHackable hackableHandle)
            {
                return new EventHacker(hackableHandle);
            }

            public EventHacker(IEventHackable hackableHandle)
            {
                this._hackableHandle = hackableHandle;
                this._hackableHandle.DisableEvents();
            }

            public void Dispose()
            {
                this._hackableHandle.EnableEvents();
            }
        }
    }
}
+2  A: 

I would use a boolean field to stop the setTotalMinutes method from being executed several times and move the creation of the eventhandlers to the InitializeComponentCustom method.

Something like this:

public partial class PollingIntervalGroupBox : GroupBox
{
    private const int DAYS_IN_WEEK = 7;
    private const int MINUTES_IN_HOUR = 60;
    private const int HOURS_IN_DAY = 24;
    private const int MINUTES_IN_DAY = MINUTES_IN_HOUR * HOURS_IN_DAY;
    private const int MAX_TOTAL_DAYS = 100;

    private static readonly decimal MIN_TOTAL_NUM_MINUTES = 1; // Anything faster than once per minute can bog down our servers.
    private static readonly decimal MAX_TOTAL_NUM_MINUTES = (MAX_TOTAL_DAYS * MINUTES_IN_DAY) - 1; // 99 days should be plenty.
    // The value above was chosen so to not cause an overflow exception.
    // Watch out for it - numericUpDownControls each have a MaximumValue setting.
    private bool _totalMinutesChanging;

    public PollingIntervalGroupBox()
    {
        InitializeComponent();
        InitializeComponentCustom();
    }

    private void InitializeComponentCustom()
    {
        this.m_upDownDays.Maximum = MAX_TOTAL_DAYS - 1;
        this.m_upDownHours.Maximum = HOURS_IN_DAY - 1;
        this.m_upDownMinutes.Maximum = MINUTES_IN_HOUR - 1;
        this.m_upDownTotalMinutes.Maximum = MAX_TOTAL_NUM_MINUTES;
        this.m_upDownTotalMinutes.Minimum = MIN_TOTAL_NUM_MINUTES;

        this.m_upDownTotalMinutes.ValueChanged += this.m_upDownTotalMinutes_ValueChanged;
        this.m_upDownDays.ValueChanged += this.m_upDownDays_ValueChanged;
        this.m_upDownHours.ValueChanged += this.m_upDownHours_ValueChanged;
        this.m_upDownMinutes.ValueChanged += this.m_upDownMinutes_ValueChanged;
    }

    private void m_upDownTotalMinutes_ValueChanged(object sender, EventArgs e)
    {
        setTotalMinutes(this.m_upDownTotalMinutes.Value);
    }

    private void m_upDownDays_ValueChanged(object sender, EventArgs e)
    {
        updateTotalMinutes();
    }

    private void m_upDownHours_ValueChanged(object sender, EventArgs e)
    {
        updateTotalMinutes();
    }

    private void m_upDownMinutes_ValueChanged(object sender, EventArgs e)
    {
        updateTotalMinutes();
    }

    private void updateTotalMinutes()
    {
        this.setTotalMinutes(
            MINUTES_IN_DAY * m_upDownDays.Value +
            MINUTES_IN_HOUR * m_upDownHours.Value +
            m_upDownMinutes.Value);
    }

    public decimal TotalMinutes { get { return m_upDownTotalMinutes.Value; } set { m_upDownTotalMinutes.Value = value; } }

    public decimal TotalHours { set { setTotalMinutes(value * MINUTES_IN_HOUR); } }

    public decimal TotalDays { set { setTotalMinutes(value * MINUTES_IN_DAY); } }

    public decimal TotalWeeks { set { setTotalMinutes(value * DAYS_IN_WEEK * MINUTES_IN_DAY); } }

    private void setTotalMinutes(decimal totalMinutes)
    {
        if (_totalMinutesChanging) return;
        try
        {
            _totalMinutesChanging = true;
            decimal nTotalMinutes = totalMinutes;
            if (totalMinutes < MIN_TOTAL_NUM_MINUTES)
            {
                nTotalMinutes = MIN_TOTAL_NUM_MINUTES;
            }
            if (totalMinutes > MAX_TOTAL_NUM_MINUTES)
            {
                nTotalMinutes = MAX_TOTAL_NUM_MINUTES;
            }
            // First set the total minutes
            this.m_upDownTotalMinutes.Value = nTotalMinutes;

            // Then set the rest
            this.m_upDownDays.Value = (int)(nTotalMinutes / MINUTES_IN_DAY);
            nTotalMinutes = nTotalMinutes % MINUTES_IN_DAY; // variable reuse.
            this.m_upDownHours.Value = (int)(nTotalMinutes / MINUTES_IN_HOUR);
            nTotalMinutes = nTotalMinutes % MINUTES_IN_HOUR;
            this.m_upDownMinutes.Value = nTotalMinutes;
        }
        finally
        {
            _totalMinutesChanging = false;
        }
    }
}
Jens Granlund
Thank you, this worked! I tried using a bool initially but messed something up. Any pointers for improvements? Where can I grab the constants such as 7, 24, 60? Does the name `InitializeComponentCustom seem right`? Anything else?
Hamish Grubijan
@Hamish, I think it´s alright to define those constants by your self. Depending on what you´r using the control for you might want to add an event to notify when the polling interval changes. I would probably inherit from `UserControl` to be able to position the updown controls easier. Otherwise I think it´s alright.
Jens Granlund
+2  A: 

This doesn't look good. The client code would have no idea why it would ever disable events on a private control that it cannot see. Nor is there any way for it to see that a public property changed, there are no events to say that a public property changed value. If it wanted to ignore change events then it would simply unsubscribe or ignore those (missing) events.

Consider the .NET framework controls as a guide. None of them have anything similar to an IEventHackable.

Hans Passant
Well, I am glad I asked this question.
Hamish Grubijan