Can someone please criticize my Factory pattern?

I recently learned more about design patterns and thought that I would hack it. I'm not sure if this is the correct way to use the factory pattern and was wondering if anyone could provide me feedback?

I basically have a set of calendar items that are similar. Nutrition, exercise and measurement. All have a date and name and can print details. but besides this, they have several items, each of which needs differently.

    public enum AppointmentType
{
    Workout,
    Meal,
    Measurement
}

public abstract class Appointment
{
    public string Name { get; set; }
    public DateTime DateStarted { get; set; }

    public virtual string PrintDetails()
    {
        return string.Format("Type: {0}\nName: {1}\nDate: {2}",this.GetType().ToString(), Name, DateStarted.ToShortDateString());
    }
}

public class Workout : Appointment
{
    public List<string> Exercises { get; set; }

    public Workout()
    {
        Exercises = new List<string>();
    }

    public override string PrintDetails()
    {
        string startInfo = base.PrintDetails();
        string addedInfo = "\nToday I will be doing the following:\n";

        foreach (string exercise in Exercises)
        {
            addedInfo += string.Format("{0}\n", exercise);
        }

        return startInfo + addedInfo;
    }
}

public class Meal : Appointment
{
    public List<string> FoodItems { get; set; }

    public Meal()
    {
        FoodItems = new List<string>();
    }

    public override string PrintDetails()
    {
        string startInfo = base.PrintDetails();
        string addedInfo = "\nToday I will be eating the following:\n";

        foreach (string foodItem in FoodItems)
        {
            addedInfo += string.Format("{0}\n", foodItem);
        }

        return startInfo + addedInfo;
    }
}

public class Measurement : Appointment
{
    public string Height { get; set; }
    public string Weight { get; set; }

    public override string PrintDetails()
    {
        string startInfo = base.PrintDetails();
        string addedInfo = string.Format("\nI am {0} feet tall and I weight {1} pounds!\n", Height, Weight);
        return startInfo + addedInfo;
    }
}

public interface IAppointmentFactory
{
    Appointment CreateAppointment(AppointmentType appointmentType);
}

public class AppointmentFactory : IAppointmentFactory
{
    public Appointment CreateAppointment(AppointmentType appointmentType)
    {
        switch (appointmentType)
        {
            case AppointmentType.Workout:
                return new Workout();
            case AppointmentType.Meal:
                return new Meal();
            case AppointmentType.Measurement:
                return new Measurement();
            default:
                return new Workout();
        }
    }
}

and here is the program using the library:

  class Program
{
    public static List<Appointment> myAppointments;
    public static AppointmentFactory factory;
    public static Appointment myAppointment;

    static void Main(string[] args)
    {
        myAppointments = new List<Appointment>();
        factory = new AppointmentFactory();
        StartLoop();
    }

    private static void StartLoop()
    {
        Console.WriteLine(string.Format("\nWelcome to Appointment App: You have {0} appointment(s)!", myAppointments.Count()));
        Console.WriteLine("0 = Exit");
        Console.WriteLine("1 = New Workout");
        Console.WriteLine("2 = New Meal");
        Console.WriteLine("3 = New Measurement");
        Console.WriteLine("P = Print Schedule\n");

        switch (Console.ReadLine().ToUpper())
        {
            case "0":
                return;
            case "1":
                CreateNewAppointment(AppointmentType.Workout);
                AddExercises();
                break;
            case "2":
                CreateNewAppointment(AppointmentType.Meal);
                AddFoodItems();
                break;
            case "3":
                CreateNewAppointment(AppointmentType.Measurement);
                GetMeasurements();
                break;
            case "P":
                PrintSchedule();
                break;
            default:
                return;
        }

        StartLoop();
    }

    private static void GetMeasurements()
    {
        Console.WriteLine("How tall are you?");
        ((Measurement)myAppointment).Height = Console.ReadLine();
        Console.WriteLine("What is your weight?");
        ((Measurement)myAppointment).Weight = Console.ReadLine();
    }

    private static void AddFoodItems()
    {
        Console.WriteLine("How many food items do you want to add?");
        string exerciseCount = Console.ReadLine();

        for (int i = 0; i < Convert.ToInt32(exerciseCount); i++)
        {
            Console.WriteLine(string.Format("Food {0}:", (i + 1)));
            ((Meal)myAppointment).FoodItems.Add(Console.ReadLine());
        }
    }

    private static void AddExercises()
    {
        Console.WriteLine("How many exercises do you want to add?");
        string exerciseCount = Console.ReadLine();

        for (int i = 0; i < Convert.ToInt32(exerciseCount); i++)
        {
            Console.WriteLine(string.Format("Exercise {0}:", (i + 1)));
            ((Workout)myAppointment).Exercises.Add(Console.ReadLine());
        }
    }

    private static void PrintSchedule()
    {
        foreach (Appointment appointment in myAppointments)
        {
            Console.WriteLine(appointment.PrintDetails());
        }
    }

    public static void CreateNewAppointment(AppointmentType appointmentType)
    {
        myAppointment = factory.CreateAppointment(appointmentType);

        Console.WriteLine("Name:");
        myAppointment.Name = Console.ReadLine();
        Console.WriteLine("Start Date:");
        myAppointment.Name = Console.ReadLine();

        myAppointments.Add(myAppointment);
    }
}

Thank!!

-ajax

+3
source share
3 answers

Here I would change it to:

public enum AppointmentType
{
    Workout,
    Meal,
    Measurement
}

public abstract class Appointment
{
    public string Name { get; set; }
    public DateTime DateStarted { get; set; }

    public abstract void PrintDetails();
    public abstract void RequestInputFromUser();
}

public class Workout : Appointment
{
    private List<string> Exercises { get; set; }

    public Workout()
    {
        Exercises = new List<string>();
    }

    public override void PrintDetails()
    {
        string startInfo = base.PrintDetails();
        string addedInfo = "\nToday I will be doing the following:\n";

        foreach (string exercise in Exercises)
        {
            addedInfo += string.Format("{0}\n", exercise);
        }

        Console.WriteLine(StartInfo + addedInfo);
    }

    public override void RequestInputFromUser()
    {
        Console.WriteLine("How many exercises do you want to add?");
        string exerciseCount = Console.ReadLine();

        for (int i = 0; i < Convert.ToInt32(exerciseCount); i++)
        {
            Console.WriteLine(string.Format("Exercise {0}:", (i + 1)));
            Exercises.Add(Console.ReadLine());
        }
    }
}

public class Meal : Appointment
{
    private List<string> FoodItems { get; set; }

    public Meal()
    {
        FoodItems = new List<string>();
    }

    public override void PrintDetails()
    {
        string startInfo = base.PrintDetails();
        string addedInfo = "\nToday I will be eating the following:\n";

        foreach (string foodItem in FoodItems)
        {
            addedInfo += string.Format("{0}\n", foodItem);
        }

        Console.WriteLine(startInfo + addedInfo);
    }

    public override void RequestInputFromUser()
    {
        Console.WriteLine("How many food items do you want to add?");
        string exerciseCount = Console.ReadLine();

        for (int i = 0; i < Convert.ToInt32(exerciseCount); i++)
        {
            Console.WriteLine(string.Format("Food {0}:", (i + 1)));
            FoodItems.Add(Console.ReadLine());
        }
    }
}

public class Measurement : Appointment
{
    private string Height { get; set; }
    private string Weight { get; set; }

    public override void PrintDetails()
    {
        string startInfo = base.PrintDetails();
        string addedInfo = string.Format("\nI am {0} feet tall and I weight {1} pounds!\n", Height, Weight);
        Console.WriteLine(startInfo + addedInfo);
    }

    public override void RequestInputFromUser()
    {
        Console.WriteLine("How tall are you?");
        Height = Console.ReadLine();
        Console.WriteLine("What is your weight?");
        Weight = Console.ReadLine();
    }
}

public interface IAppointmentFactory
{
    Appointment CreateAppointment(AppointmentType appointmentType);
}

public class AppointmentFactory : IAppointmentFactory
{
    public Appointment CreateAppointment(AppointmentType appointmentType)
    {
        Appointment apt;

        switch (appointmentType)
        {
            case AppointmentType.Workout:
                apt = new Workout();
            case AppointmentType.Meal:
                apt = new Meal();
            case AppointmentType.Measurement:
                apt = new Measurement();
            default:
                apt = new Workout();
        }

        Console.WriteLine("Name:");
        apt.Name = Console.ReadLine();
        Console.WriteLine("Start Date:");
        apt.Name = Console.ReadLine();    // Logic error on this line.
                                          // Change it to do what you mean.

        apt.RequestInputFromUser();

        return apt;
    }
}

using the following code:

class Program
{
    public static List<Appointment> myAppointments;
    public static AppointmentFactory factory;
    public static Appointment myAppointment;

    static void Main(string[] args)
    {
        myAppointments = new List<Appointment>();
        factory = new AppointmentFactory();
        StartLoop(factory);
    }

    private static void StartLoop(IAppointmentFactory factory)
    {
        bool exit = false;

        while(!exit)
        {

            Console.WriteLine(string.Format("\nWelcome to Appointment App: You have {0} appointment(s)!", myAppointments.Count()));
            Console.WriteLine("0 = Exit");
            Console.WriteLine("1 = New Workout");
            Console.WriteLine("2 = New Meal");
            Console.WriteLine("3 = New Measurement");
            Console.WriteLine("P = Print Schedule\n");

            switch (Console.ReadLine().ToUpper())
            {
                case "0":
                    exit = true;
                    break;
                case "1":
                    myAppointments.Add( factory.CreateAppointment( AppointmentType.Workout ) );
                    break;
                case "2":
                    myAppointments.Add( factory.CreateAppointment( AppointmentType.Meal ) );
                    break;
                case "3":
                    myAppointments.Add( factory.CreateAppointment( AppointmentType.Measurement ) );
                    break;
                case "P":
                    PrintSchedule();
                    break;
                default:
                    exit = true;
            }

        }

    }

    private static void PrintSchedule()
    {
        foreach (Appointment appointment in myAppointments)
        {
            appointment.PrintDetails();
        }
    }

}

A few notes:

  • ( ).
  • ( )
  • ( )

, . , , - . .

StartLoop(). , , , .

EDIT: Factory.

:

  • , , , IO. , , Appointment. -. , IO, Appointment
  • , . , . Clean Code. , .

, :)

+3

, factory , . , , factory, vends

Supersize-Workout
Supersize-Meal
Supersize-Measurement

, vends

Mini-Workout
Mini-Meal
Mini-Measurement

factory , , , , -, .

factory factory .

0

, - .

( , ):

  • OCP. , / , , () . , , / - , .

  • Factory , factory, ( OCP). - .

- .

0

Source: https://habr.com/ru/post/1757887/


All Articles