Refactoring a list of objects to implement a business rule

I need to refactor the following class:

public interface IEmployee
{
    int VacationWeeks { get; }
    int YearsWithCompany { set; get; }
    double Salary { set; get; }
}

public class Employee : IEmployee
{
    private readonly int vacationWeeks;

    public Employee(int vacationWeeks)
    {
        this.vacationWeeks = vacationWeeks;
    }

    public int VacationWeeks
    {
        get { return vacationWeeks; }
    }

    public int YearsWithCompany { set; get; }
    public double Salary { set; get; }
}

I need to make sure that VacationWeeks depends only on YearsWithCompany, and I am loading the mapping from the database. So far I have come up with this:

public class EmployeeNew : IEmployee
{
    private Dictionary<int,int> vacationWeeksTable;

    public EmployeeNew(Dictionary<int, int> vacationWeeksTable)
    {
        this.vacationWeeksTable = vacationWeeksTable;
    }

    public int VacationWeeks
    {
        get { return vacationWeeksTable[YearsWithCompany]; }
    }

    public int YearsWithCompany { set; get; }
    public double Salary { set; get; }
}

This class implements what I want, but it still has one vulnerability: different instances of EmployeeNew in the same collection may have been created with different instances of vacationWeeksTable.

All instances of EmployeeNew in the same collection must refer to the same vacationWeeksTable.

The application I am refactoring uses lots of List all over the system, and we need to be able to modify YearsWithCompany and Salary, yet to guarantee that only one vacationWeeksTable is used per List. These lists are iterated several times; its elements are modified in each iteration.

Here is my imperfect solution. Suggestions are welcome:

// this class does two things, which I do not like
    public class EmployeeList : IEnumerable<IEmployee>, IEmployee
    {
        private Dictionary<int, int> vacationWeeksTable;
        private List<EmployeeSpecificData> employees;
        private int currentIndex;
        private EmployeeSpecificData CurrentEmployee
        {
            get { return employees[currentIndex]; }
        }

        public IEnumerator<IEmployee> GetEnumerator()
        {
            for (currentIndex = 0; currentIndex < employees.Count; currentIndex++)
            {
                yield return this;
            }
        }

        IEnumerator IEnumerable.GetEnumerator()
        {
            return GetEnumerator();
        }

        public int VacationWeeks
        {
            get { return vacationWeeksTable[YearsWithCompany]; }
        }

 开发者_开发技巧       // this is ugly repetitive code I don't like
        public int YearsWithCompany
        {
            get { return CurrentEmployee.YearsWithCompany; }
            set { CurrentEmployee.YearsWithCompany = value; }
        }

        // this is ugly repetitive code I don't like
        public double Salary
        {
            get { return CurrentEmployee.Salary; }
            set { CurrentEmployee.Salary = value; }
        }


    }

I use the following to create and init some of the classes that need default and shared behaviour. Maybe if you can refactor it will help:

It is some form of the Factory and FlyWeight patterns combined (the flyweight part can be removed in your scenario), which in addition has a concept of class Type shared handlers.

I simplified and removed some stuff that you wont need but there is more to remove, I added comments.

Usage would be: (app init)

Dictionary<int,int> vacationWeeksTable = new Dictionary<int,int>();
// fill the table
Factory<Employee>.Init(vacationWeeksTable);

The whenever you create a Employee class:

// remove grouping in the factory class to remove this null
Employee em = Factory<Employee>.Create(null);

It takes only a WeakReference to the classes so you don't have to worry about GC.

Each employee will have the shared vacationWeeksTable setup on creation, without the possibility to change it after from outside if not using the factory class.

You could change the vacation table for all running instances of Employee at any moment in the runtime of the app with:

// this will call the method registered for SetInitialdata on all instances of Employee classes.
// again remove grouping to remove that null
Factory<Employee>.Call(EventHandlerTypes.SetInitialData, null, vacTable);

Sample implementation of Employee:

class Employee : IBaseClass
{
    private Dictionary<int, int> vacationWeeksTable;

    public virtual void RegisterSharedHandlers(int? group, Action<IKey, int?, EventHandlerTypes, Action<object, SharedEventArgs>> register)
    {
        group = 0; // disable different groups
        register(new Key<Employee, int>(0), group, EventHandlerTypes.SetInitialData, SetVacationWeeksTable);
    }

    public virtual void RegisterSharedData(Action<IKey, object> regData)
    {
        // remove this from factory and interface, you probably dont need it
        // I have been using it as a FlyWeight data store for classes.            
    }

    private void SetVacationWeeksTable(object sender, SharedEventArgs e)
    {
        vacationWeeksTable = e.GetData<Dictionary<int, int>>();
    }

}

Code pattern Implementation:

IBaseClass : interface that each of my classes that are creatable through a factory implement

public enum EventHandlerTypes
{
    SetInitialData // you can add additional shared handlers here and Factory<C>.Call - it.
}

public class SharedEventArgs : EventArgs
{
    private object data;

    public SharedEventArgs(object data)
    {
        this.data = data;
    }

    public T GetData<T>()
    {
        return (T)data;
    }
}

public interface IBaseClass
{
    void RegisterSharedHandlers(int? group, Action<IKey, int?, EventHandlerTypes, Action<object, SharedEventArgs>> regEvent);
    void RegisterSharedData(Action<IKey, object> regData);
}

Utility generic classes:

public interface IKey
{
    Type GetKeyType();
    V GetValue<V>();
}

public class Key<T, V> : IKey
{
    public V ID { get; set; }
    public Key(V id)
    {
        ID = id;
    }

    public Type GetKeyType()
    {
        return typeof(T);
    }

    public Tp GetValue<Tp>()
    {
        return (Tp)(object)ID;
    }
}

public class Triple<T, V, Z>
{
    public T First { get; set; }
    public V Second { get; set; }
    public Z Third { get; set; }
    public Triple(T first, V second, Z third)
    {
        First = first;
        Second = second;
        Third = third;
    }
}

Factory class with slight modification to handle your scenario:

   public static class Factory<C> where C : IBaseClass, new()
    {
        private static object initialData;
        private static Dictionary<IKey, Triple<EventHandlerTypes, int, WeakReference>> handlers = new Dictionary<IKey, Triple<EventHandlerTypes, int, WeakReference>>();
        private static Dictionary<IKey, object> data = new Dictionary<IKey, object>();

        static Factory()
        {
            C newClass = new C();
            newClass.RegisterSharedData(registerSharedData);
        }

        public static void Init<IT>(IT initData)
        {
            initialData = initData;
        }

        public static Dt[] GetData<Dt>()
        {
            var dataList = from d in data where d.Key.GetKeyType() == typeof(Dt) select d.Value;

            return dataList.Cast<Dt>().ToArray();
        }

        private static void registerSharedData(IKey key, object value)
        {
            data.Add(key, value);
        }

        public static C Create(int? group)
        {
            C newClass = new C();
            newClass.RegisterSharedHandlers(group, registerSharedHandlers);
            // this is a bit bad here since it will call it on all instances
            // it would be better if you can call this from outside after creating all the classes
            Factory<C>.Call(EventHandlerTypes.SetInitialData, null, initialData);
            return newClass;
        }

        private static void registerSharedHandlers(IKey subscriber, int? group, EventHandlerTypes type, Action<object, SharedEventArgs> handler)
        {
            handlers.Add(subscriber, new Triple<EventHandlerTypes, int, WeakReference>(type, group ?? -1, new WeakReference(handler)));
        }

        public static void Call<N>(EventHandlerTypes type, int? group, N data)
        {
            Call<N>(null, type, group, data);
        }

        public static void Call<N>(object sender, EventHandlerTypes type, int? group, N data)
        {
            lock (handlers)
            {
                var invalid = from h in handlers where h.Value.Third.Target == null select h.Key;

                // delete expired references
                foreach (var inv in invalid.ToList()) handlers.Remove(inv);

                var events = from h in handlers where h.Value.First == type && (!@group.HasValue || h.Value.Second == (int)@group) select h.Value.Third;

                foreach (var ev in events.ToList())
                {
                    // call the handler
                    ((Action<object, SharedEventArgs>)ev.Target)(sender, arg);
                }
            }
        } 
    }

Make a class which contains a Dictionary. Creating or getting instance of this new class will load the dictionary in a consistent way. Then your BOs can take an instance of the class, thus ensuring they're all using the same data (because the class containingthe list knows how to load itself with the proper set of data).