Диспенсер для кофе OO

Эта картина вдохновила конкуренцию нескольких моих друзей на переписывание этого кода в более подходящем стиле OO.

 кружка кофе с кодом

Это то, что я придумал. Любые мысли:

public enum CoffeeKind
{
    Black = 1,
    Cappuccino,
    Espresso
}

public class Coffee
{
    //Measured in celcius
    public int Temprature { get; set; } = 30;
    public bool HasMilk { get; set; }
    public int Sugar { get; set; }
    public CoffeeKind Kind { get; set; }
    public bool Cold => Temprature < 24;
}

public enum CupSize
{
    Small = 1,
    Medium,
    Large,
    ExtraLarge
}

public class Cup
{
    public static Cup SMALL = new Cup(CupSize.Small);
    public static Cup MEDIUM = new Cup(CupSize.Medium);
    public static Cup LARGE = new Cup(CupSize.Large);
    public static Cup EXTRA_LARGE = new Cup(CupSize.ExtraLarge);

    public static Dictionary<CupSize, Cup> Cups = new Dictionary<CupSize, Cup>
    {
        [CupSize.Small] = SMALL,
        [CupSize.Medium] = MEDIUM,
        [CupSize.Large] = LARGE,
        [CupSize.ExtraLarge] = EXTRA_LARGE
    };

    public CupSize Size { get; }
    public int Amount { get; set; }
    public bool Empty => Amount == 0;
    public Coffee Coffee { get; set; }

    private Cup() : this(CupSize.Medium) { }
    private Cup(CupSize size)
    {
        Size = size;
    }

    public void Fill(CoffeeKind kind)
    {
        if (Empty)
        {
            Coffee = new Coffee { Kind = kind };
            Amount = SetAmountBySize(Size);
        }
    }

    private static int SetAmountBySize(CupSize size)
    {
        switch (size)
        {
            case CupSize.Small:
                return 100;
            case CupSize.Large:
                return 300;
            case CupSize.ExtraLarge:
                return 400;
            case CupSize.Medium:
            default:
                return 200;
        }
    }
}

public class Person
{
    public string FirstName { get; set; }
    public string LastName { get; set; }
    public string Name => $"{FirstName} {LastName}";
    public Cup Cup { get; set; }

    public Person() { }
    public Person(string firstName, string lastName)
    {
        FirstName = firstName;
        LastName = lastName;
    }

    public void GetCoffee(CupSize size, CoffeeKind kind)
    {
        Cup = Cup.Cups[size];
        Cup.Fill(kind);
    }

    public void Drink()
    {
        if (Cup?.Empty == false && Cup?.Coffee?.Cold == false)
        {
            Cup.Amount = 0;
        }
    }

    public void Drink(int amount)
    {
        if (Cup?.Empty == false && Cup?.Coffee?.Cold == false)
        {
            Cup.Amount -= amount;
        }
    }
}

class Program
{
    const int EXIT = -1;
    const int GET_COFFEE = 1;
    const int DRINK_COFFEE = 2;
    const int DRINK_AMOUNT = 3;

    //TODO change these to use values of enum - or just change the calls to them
    const int SMALL = 1;
    const int MEDIUM = 2;
    const int LARGE = 3;
    const int EXTRA_LARGE = 4;

    const int BLACK = 1;
    const int CAPPUCCINO = 2;
    const int ESPRESSO = 3;

    static Person person;
    static void Main(string[] args)
    {
        Console.WriteLine("Enter first name:");
        string firstName = Console.ReadLine();
        Console.WriteLine("Enter last name:");
        string lastName = Console.ReadLine();

        person = new Person(firstName, lastName);

        var choice = GetUserAction();
        while (choice != EXIT)
        {
            if (choice == GET_COFFEE)
            {
                if (person?.Cup?.Empty == false)
                {
                    Console.WriteLine("Old coffee will be thrown out");
                }
                int cupSize = GetCupSize();
                int coffeeKind = GetCoffeKind();

                person.GetCoffee((CupSize)cupSize, (CoffeeKind)coffeeKind);
                Console.WriteLine($"Got a {person.Cup.Size.ToString()} size {person.Cup.Coffee.Kind.ToString()} coffee");
            }
            else if (choice == DRINK_COFFEE)
            {
                if (person?.Cup?.Empty == false)
                {
                    person.Drink();
                }
                else
                {
                    Console.WriteLine("Can not drink without coffee!");
                }
            }

            choice = GetUserAction();
        }
    }

    private static int GetCoffeKind()
    {
        Console.WriteLine($"{BLACK}: Black");
        Console.WriteLine($"{CAPPUCCINO}: Cappuccino");
        Console.WriteLine($"{ESPRESSO}: Espresso");

        var result = int.Parse(Console.ReadLine());

        if (result < BLACK || result > ESPRESSO)
        {
            return GetCoffeKind();
        }

        return result;
    }

    private static int GetCupSize()
    {
        Console.WriteLine($"{SMALL}: {nameof(SMALL)}");
        Console.WriteLine($"{MEDIUM}: Medium");
        Console.WriteLine($"{LARGE}: Large");
        Console.WriteLine($"{EXTRA_LARGE}: Extra Large");
        var result = int.Parse(Console.ReadLine());

        if (result < SMALL || result > EXTRA_LARGE)
        {
            return GetCupSize();
        }

        return result;
    }

    private static int GetUserAction()
    {
        Console.WriteLine("What would you like to do:");
        Console.WriteLine($"{GET_COFFEE}: Get coffee");
        // You can't drink coffee without a cup
        if (person?.Cup != null)
        {
            Console.WriteLine($"{DRINK_COFFEE}: Drink coffee");
            Console.WriteLine($"{DRINK_AMOUNT}: Drink some coffee");
        }
        Console.WriteLine($"{EXIT}: Exit");
        return int.Parse(Console.ReadLine());
    }
}
35 голосов | спросил Mord Zuber 2 FebruaryEurope/MoscowbTue, 02 Feb 2016 18:14:10 +0300000000pmTue, 02 Feb 2016 18:14:10 +030016 2016, 18:14:10

4 ответа


26

Вы злоупотребляете оператором распространения пустоты. Например.

person = new Person(firstName, lastName);

var choice = GetUserAction();
while (choice != EXIT)
{
    if (choice == GET_COFFEE)
    {
        if (person?.Cup?.Empty == false)
        {

Невозможно, чтобы этот человек был null. После создания вы никогда не ставите его ни на что другое. Как ни странно, я думал, что это будет моя самая используемая функция C # 6, но я почти никогда не использую ее.


C # обычно никогда не использует SHOUT_CASE. Ваши константы должны быть PascalCase.


Если вы собираетесь установить явное значение для одного члена перечисления, я бы сказал, что вы должны сделать это для всех:

public enum CoffeeKind
{
    Black = 1,
    Cappuccino = 2,
    Espresso = 3
}

Я бы сказал, что ваша модель немного отсюда. Эспрессо - черный кофе, т. Е. В нем нет молока.

public enum DrinkType
{
    FilterCoffee,
    Cappuccino,
    Espresso
}

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


Я не вижу смысла в этих полях:

public static Cup SMALL = new Cup(CupSize.Small);
public static Cup MEDIUM = new Cup(CupSize.Medium);
public static Cup LARGE = new Cup(CupSize.Large);
public static Cup EXTRA_LARGE = new Cup(CupSize.ExtraLarge);

У вас уже есть перечисление - убедитесь, что вы правильно применили равенство, и пусть люди создают свою чашку чата .


Как указано выше:

public static Dictionary<CupSize, Cup> Cups = new Dictionary<CupSize, Cup>
{
    [CupSize.Small] = SMALL,
    [CupSize.Medium] = MEDIUM,
    [CupSize.Large] = LARGE,
    [CupSize.ExtraLarge] = EXTRA_LARGE
};

Если вы хотите узнать о чашках, вам нужно иметь шкафчик:

public class Cupboard : IRepository<Cup>
{
    public Cup GetCupForPerson(Person p)
    {
        // ...
    }
}

Тебе нравится, когда кофе холодно;)

public class Coffee
{
    //Measured in celcius
    public int Temprature { get; set; } = 30;

Это нечетный метод:

public void Drink()
{
    if (Cup?.Empty == false && Cup?.Coffee?.Cold == false)
    {
        Cup.Amount = 0;
    }
}

Почему имеет значение, если чаша пуста?

  1. Если вы устанавливаете сумму от 0 до 0, никто даже не заметит
  2. Я регулярно отвлекаюсь и пытаюсь выпить из пустой чашки (иногда даже вчера).

Вы должны либо TryDrink, если это действие может не произойти. Или, если вы думаете, что ваш человек (как и я) собирается совершить пить все, что есть в этой чаше, тогда вы всегда должны позволять им это делать.


Остерегайтесь опечаток: GetCoffeKind должен быть GetCoffeeKind.

ответил RobH 2 FebruaryEurope/MoscowbTue, 02 Feb 2016 19:08:02 +0300000000pmTue, 02 Feb 2016 19:08:02 +030016 2016, 19:08:02
11

Некоторые быстрые замечания

int GetCupSize(), int GetUserAction() и int GetCoffeKind()

Использование int.TryParse() было бы лучше, чтобы любой неверный введенный символ не выдавал исключение.

int SetAmountBySize(CupSize size)

Здесь вы действительно не устанавливаете сумму. Попытайтесь найти лучшее имя.

Общие

Вместо if (someBoolean == false) вы должны использовать более идиоматический способ, например, if (!someBoolean).

ответил Heslacher 2 FebruaryEurope/MoscowbTue, 02 Feb 2016 18:45:07 +0300000000pmTue, 02 Feb 2016 18:45:07 +030016 2016, 18:45:07
10

Я бы сказал, что есть ошибка в логике кода в чашке :). Это должно быть больше похоже на следующее:

Cup coffeeCup = new Cup(size);

while(true)
{
    if(cup.IsEmpty)
    {
        cup.Refill(coffeeType);
    }

    cup.Drink();
}

Что касается вашего кода, я бы просто удалил следующие строки:

public static Cup SMALL = new Cup(CupSize.Small);
public static Cup MEDIUM = new Cup(CupSize.Medium);
public static Cup LARGE = new Cup(CupSize.Large);
public static Cup EXTRA_LARGE = new Cup(CupSize.ExtraLarge);

, поскольку они не добавляют ничего значимого ИМО.

ответил Gentian Kasa 2 FebruaryEurope/MoscowbTue, 02 Feb 2016 19:07:37 +0300000000pmTue, 02 Feb 2016 19:07:37 +030016 2016, 19:07:37
7

Мой комментарий:

  

OO-мудрый, это пролитая чашка. Это повсюду. У вас есть все, кроме CupOfCoffee. «Дозатор» в названии может быть выведен только после разлива.

     

Max:   @radarbob, пожалуйста, спросите пожалуйста?

static void Main(string[] args){
    Person theDeveloper = new Person();
    Dispenser coffeeMaker = new Dispenser();

    CoffeeOrder whatIWant = GetUserAction();
    CupOfCoffee myCuppa = CoffeeMaker.Brew(whatIWant);
    theDeveloper.Drink(myCuppa);
}

Объектно-ориентированный, перечисляемый

  • Структура

  • Инкапсуляция
  • Инверсия управления

  • Моделирование домена. На ОП есть дозатор и чашка кофе.

  • Единый принцип ответственности - да, это относится к методам.

  • При создании этого кода не пострадали никакие свойства.

    • Алан Кей сказал, перефразируя: «Если вы настраиваете состояние напрямую, не используя методы, вы делаете это неправильно. «
  • OO является фрактальным - не оставляйте применение дизайна только потому, что:

    • «Это не много кода»
    • «Метод мал, мы можем добавить здесь больше кода»
    • Посмотрите! Класс. Это ОО! Это ТВЕРДОЕ! Пусть начнется код barf.
  • Реальность, прервана:

    • //TODO change these... у нас более 700 из них в нашей базе production . Код походит на быстросохнущий цемент при выпуске. он твердый (каламбур).
    • Это было просто быстро брошено вместе
      • Был там, сделал это. И я лично видел этот манифест в качестве разницы между проведением 2-х дней в формальном тесте и 3-месячным циклом испытания-неудачи в аду.
ответил radarbob 3 FebruaryEurope/MoscowbWed, 03 Feb 2016 18:31:34 +0300000000pmWed, 03 Feb 2016 18:31:34 +030016 2016, 18:31:34

Похожие вопросы

Популярные теги

security × 330linux × 316macos × 2827 × 268performance × 244command-line × 241sql-server × 235joomla-3.x × 222java × 189c++ × 186windows × 180cisco × 168bash × 158c# × 142gmail × 139arduino-uno × 139javascript × 134ssh × 133seo × 132mysql × 132