Может кто-нибудь критиковать мою фабричную модель? - PullRequest
3 голосов
/ 03 августа 2010

Недавно я узнал больше о шаблонах проектирования и подумал, что мне стоит попробовать его. Я не уверен, является ли это правильным способом использования фабричного шаблона, и мне было интересно, если кто-то может предоставить мне обратную связь?

У меня в основном набор элементов календаря, которые похожи. Питание, тренировки и замеры. Все они имеют дату и имя и могут печатать детали. но кроме этого у них есть несколько предметов, каждый из которых нужен по-своему.

    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();
        }
    }
}

и вот программа, использующая библиотеку:

  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);
    }
}

Спасибо !!

-ajax

Ответы [ 3 ]

3 голосов
/ 03 августа 2010

Вот что я бы изменил:

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;
    }
}

с кодом, использующим его следующим образом:

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();
        }
    }

}

Несколько вещей, на которые стоит обратить внимание:

  • Все свойства в низших классах являются частными (хорошая вещь)
  • Нет приведения типов (еще одна хорошая вещь)
  • Легко добавить другой тип встречи (еще одна хорошая вещь)

Есть еще вещи, которые могут измениться. Базовый класс, имеющий общедоступные свойства, все еще плох. Но это большое улучшение.

Я также удалил рекурсивный вызов StartLoop (). Вы не хотите увеличивать стек, когда все, что вам нужно, это цикл.

РЕДАКТИРОВАТЬ: Внесены некоторые изменения в класс Factory.

В ответ на комментарии:

  • Чтобы удалить зависимость от консоли, создайте интерфейс, который представляет то, что вы хотите сделать с IO. Создайте классы, которые реализуют этот интерфейс, и отправьте экземпляр в класс Appointment. Оттуда, чтобы все использовали экземпляр двигателя ввода-вывода. Чтобы изменить платформу, измените класс, реализующий ввод-вывод, который отправляется в класс назначений
  • Открытые свойства являются плохими по той же причине, что публичные переменные являются плохими. Класс должен скрывать свою реализацию, а открытые свойства предоставляют реализацию. Для более подробной информации предлагаю Чистый код . Он описывает это намного лучше, чем я.

В целом, неплохо:)

0 голосов
/ 03 августа 2010

Ну, для начала - ваш код хорошо следует шаблону.

Ваша конкретная реализация страдает от нескольких проблем (но ничего не может быть обойдено):

  1. Использование перечисления для параметра нарушает принцип OCP .Если вы добавите или еще хуже, удалите значение в / из перечисления, клиенты должны будут перекомпилировать, чтобы использовать правильное (новое) перечисление.Для ответа из учебника - вы можете использовать строку для представления типа, который вы хотите создать, а затем генерировать исключения / обрабатывать их другим способом, когда запрашивается неподдерживаемый тип.

  2. Добавление новой реализации Factory будет вынуждено поддерживать только те типы возвращаемых данных, которые могут обрабатывать исходные фабрики - если только вы не можете перекомпилировать (снова нарушая OCP).В зависимости от вашего сценария - это может быть хорошо.

Но кроме этого - по моему мнению, это жизнеспособная реализация.

0 голосов
/ 03 августа 2010

Насколько я понимаю, абстрактный шаблон фабрики используется, когда вам нужно создать разные наборы объектов одного типа. Так, например, у вас может быть фабрика, которая продает

Supersize-Workout
Supersize-Meal
Supersize-Measurement

и еще один, который торгует

Mini-Workout
Mini-Meal
Mini-Measurement

Эти фабричные классы могут затем передаваться в систему, которая должна создавать тренировки, приемы пищи и измерения, но не заботится о том, являются ли они большими или миниатюрными, если они все одинаковые.

В отсутствие второй фабрики я считаю вашу абстрактную фабрику немного странной.

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...