enis
enis

Reputation: 105

Best Implementation to avoid if/else

I am doing a program for vacation in a company and the time that is allowed to be in a specific holiday.

I used an Abstract class with an abstract method :

public abstract class Abstract : TimeLength
{
    public AbstractTest(string employeeCode, string employee, string typeOfHoliday, DateTime startDate, DateTime endDate) : base(startDate, endDate, "")
    {
        TypeOfHoliday = typeOfHoliday;
        Employee = employee;
        EmployeeCode = employeeCode;
    }
    public string EmployeeCode { get; set; }
    public string Employee { get; set; }
    public string TypeOfHoliday { get; set; }

    public abstract bool HolidayValidation(string typeOfHoliday);
}

And I used multiple class that inherent from this abstract class like this two :

class MarriageVacation : Abstract
{
    public MarriageVacation(string employeeCode, string employee, string typeOfHoliday, DateTime startDate, DateTime endDate) : base(employeeCode, employee, typeOfHoliday, startDate, endDate)
    {
    }

    public override bool HolidayValidation(string typeOfHoliday)
    {
        if (Days() > (int)Holiday.MarriageVacation)
        {
            MessageBox.Show("Marriage Vacation Can only be 5 Days");
            return false;
        }
        else
            return true;
    }
}


class Bereavement : Abstract
{
    public Bereavement(string employeeCode, string employee, string typeOfHoliday, DateTime startDate, DateTime endDate) : base(employeeCode, employee, typeOfHoliday, startDate, endDate)
    {
    }

    public override bool HolidayValidation(string typeOfHoliday)
    {
        if (Days() > (int)Holiday.Bereavement)
        {
            MessageBox.Show("Bereavement Can only be 5 Days");
            return false;
        }
        else
            return true;
    }
}

I use Enum for holidays and in the main I want to register this based on the users choice like this :

List<Abstract> holiday = new List<Abstract>();
if(CmbTypeHolidays.Text == Holiday.Bereavement.ToString())
        {
            var holid = new Bereavement(CmbEmpHolidays.Text.Split('-')[0], CmbEmpHolidays.Text.Split('-')[1], CmbTypeHolidays.Text, Convert.ToDateTime(StartDateHolidays.Value), Convert.ToDateTime(EndDateHolidays.Value));

            if (holid.HolidayValidation(CmbTypeHolidays.Text))
            {
                holiday.Add(holid);
                var bindingList = new BindingList<Abstract>(holiday);
                dataGridHolidays.DataSource = bindingList;
                controlPanelHolidays.Visible = false;
            }
        }
        else if (CmbTypeHolidays.Text == Holiday.MarriageVacation.ToString())
        {
            var holid = new MarriageVacation(CmbEmpHolidays.Text.Split('-')[0], CmbEmpHolidays.Text.Split('-')[1], CmbTypeHolidays.Text, Convert.ToDateTime(StartDateHolidays.Value), Convert.ToDateTime(EndDateHolidays.Value));

            if (holid.HolidayValidation(CmbTypeHolidays.Text))
            {
                holiday.Add(holid);
                var bindingList = new BindingList<Abstract>(holiday);
                dataGridHolidays.DataSource = bindingList;
                controlPanelHolidays.Visible = false;
            }
        }

I wanted to know a better way to implement this solution or just to change the code that inserts data to the abstract List

Upvotes: 3

Views: 148

Answers (3)

kkica
kkica

Reputation: 4104

You have the same(almost) implementation for HolidayValidation and you dont use typeOfHoliday.

From what you have posted you might as well add the Holiday enum as parameter and property to base class(Abstract) and not have any inheritance at all. Implement the HolidayValidation in the base class and use the Holiday property to compare to Days

Upvotes: 1

enis
enis

Reputation: 105

I made this changes based on the answers on this questions this is the Main class (Abstract) :

public class AbstractTest : TimeLength
{
    public AbstractTest(string employeeCode, string employee, Holiday typeOfHoliday, DateTime startDate, DateTime endDate) : base(startDate, endDate, "")
    {
        TypeOfHoliday = typeOfHoliday;
        Employee = employee;
        EmployeeCode = employeeCode;
    }
    public string EmployeeCode { get; set; }
    public string Employee { get; set; }
    public Holiday TypeOfHoliday { get; set; }

    public bool HolidayValidation(Holiday typeOfHoliday)
    {
        return Days() > (int)typeOfHoliday;
    }
}

And in the Main i changed into this :

Holiday MyStatus = (Holiday)Enum.Parse(typeof(Holiday), CmbTypeHolidays.Text, true);
        var holid = new AbstractTest(CmbEmpHolidays.Text.Split('-')[0], CmbEmpHolidays.Text.Split('-')[1], MyStatus, Convert.ToDateTime(StartDateHolidays.Value), Convert.ToDateTime(EndDateHolidays.Value));

        if (!holid.HolidayValidation(MyStatus))
        {
            holiday.Add(holid);
            var bindingList = new BindingList<AbstractTest>(holiday);
            dataGridHolidays.DataSource = bindingList;
            controlPanelHolidays.Visible = false;
        }
        else
        {
            MessageBox.Show($"{holid.TypeOfHoliday} Cant be more than {(int)MyStatus} Days");
        }

For the typeOfHoliday i used Holiday type so it is easier to work with and the choice that the user makes i convert it to Enum Holiday

Upvotes: 1

Sergey Kalinichenko
Sergey Kalinichenko

Reputation: 727067

You will need to set up a factory that maps holiday type name to the class implementing it:

private class HolidayConstructorArgs {
    public string EmployeeCode {get;set;}
    public string Employee {get;set;}
    public string TypeOfHoliday {get;set;}
    public DateTime From {get;set;}
    public DateTime To {get;set;}
}

private static readonly IDictionary<string,Func<HolidayConstructorArgs,AbstractHoliday>> HolidayByTypeCode =
    new Dictionary<string,Func<HolidayConstructorArgs,AbstractHoliday>> {
        [$"{Holiday.Bereavement}"] = a => new Bereavement(a.EmployeeCode, a.Employee, a.TypeOfHoliday, a.From, a.To)
    ,   [$"{Holiday.MarriageVacation}"] = a => new MarriageVacation(a.EmployeeCode, a.Employee, a.TypeOfHoliday, a.From, a.To)
    };

Now you can get the factory from the dictionary, and use it to instantiate the object:

if (HolidayByTypeCode.TryGetValue(CmbTypeHolidays.Text, out var factory)) {
    // This is where the "magic" happens:
    // Func<> will invoke the appropriate constructor without a conditional
    var holid = factory(
        new HolidayConstructorArgs {
            EmployeeCode = CmbEmpHolidays.Text.Split('-')[0]
        ,   Employee = CmbEmpHolidays.Text.Split('-')[1]
        ,   TypeOfHoliday = CmbTypeHolidays.Text
        ,   From = Convert.ToDateTime(StartDateHolidays.Value)
        ,   To = Convert.ToDateTime(EndDateHolidays.Value)
        }
    );
    // ... The rest of your code remains the same
}

Upvotes: 3

Related Questions