С# Рациональное написание кода.

N

Nikitos704

Client
Регистрация
08.06.2016
Сообщения
54
Благодарностей
9
Баллы
8
Приветствую.
Делаю кубик свой с# код.

Подскажите как тут правильно и грамотно сделать:
Есть класс Exchange(Биржи). Далее есть 10 объектов данного класса. У каждого объекта одинаковый набор переменных, но метод должен быть у каждого объекта свой. Подскажите как сделать так, чтобы у каждого экземпляра класса был свой метод.

Можно конечно в методе Action добавить 10 условий и сравнивать название биржи и в зависимости от этого выполнять код, но мне кажется это нерациональное решение. Еще очень важно сохранить структуру, т.е. чтобы был массив объектов для того чтобы можно было их перебирать в цикле.

Код: 
    class Exchange
    {
        public int id;
        public string name;
        public int timeout;
        public int next_zahod;
        public string link;

        public void Action()
        {
            Console.WriteLine("Выполняется действие для биржи X");
        }

    }
   
   
   
    ExchangeList[] Exchanges = new ExchangeList[10];
    Exchanges[0] = new ExchangeList();

    Exchanges[0].id = 0;
    Exchanges[0].name = "exmo";
    Exchanges[0].timeout = 240;
    Exchanges[0].link = "http://exmo.ru";

    for( int i=0; i < 10; i++)
    if (подошло время для биржи)
        Exchanges[i].Action(); // выполняем действия для данной биржи
 
amyboose

amyboose

Client
Регистрация
21.04.2016
Сообщения
2 312
Благодарностей
1 191
Баллы
113
1) создать абстрактный класс / интерфейс
2) унаследоваться от него
3) реализовать свои методы в каждом классе
 
  • Спасибо
Реакции: Nikitos704
N

Nikitos704

Client
Регистрация
08.06.2016
Сообщения
54
Благодарностей
9
Баллы
8
Сделал так:

Код: 
    class Person
    {
        //private string _firstName;
        //private string _lastName;

        public string _firstName;
        public string _lastName;
        public int id;
        public string name;

        public string FirstName
        {
            get { return _firstName; }
            set { _firstName = value; }
        }
        public string LastName
        {
            get { return _lastName; }
            set { _lastName = value; }
        }
        public void Display()
        {
            Console.WriteLine($"{FirstName} {LastName}");
        }
    }
    class Employee1 : Person
    {
        public void Display1()
        {
            Console.WriteLine("1-й наследник");
        }
    }
    class Employee2 : Person
    {
        public void Display1()
        {
            Console.WriteLine("2-й наследник");
        }
    }
  


            ArrayList myArrayList = new ArrayList();
            myArrayList.Add(new Employee1());
            myArrayList.Add(new Employee2());

            ((Employee1)myArrayList[0]).id = 10;
            ((Employee2)myArrayList[1]).id = 20;

            ((Employee1)myArrayList[0]).name = "1-й";
            ((Employee2)myArrayList[1]).name = "2-й";

            foreach (object cl in myArrayList)
            {
                if (cl is Employee1)
                {
                    Console.WriteLine(((Employee1)cl).id);
                    ((Employee1)cl).Display1();
                }
                if (cl is Employee2)
                {
                    Console.WriteLine(((Employee2)cl).name);
                    ((Employee2)cl).Display1();
                }
            }

Вроде бы цель достигнута. Но все равно как-то громоздко. Есть общий класс (в нем общие переменные для всех дочерних классов). Есть наследники (2 класса), в каждом наследнике есть метод Display1 и в каждом наследнике он выполняет свою функцию.

Следующая часть кода отвечает за создание массива, тип каждого элемента в массиве будет свой (Employee1, Employee2) и т.д. Заполняю поля элементов массива значениями. И последний цикл. В котором перебираем элементы массива и если тип элемента массива соответствует дочернему классу, то выполняем метод Display1() данного дочернего класса.

Мне код кажется громоздким, его можно как-то подсократить (упростить) без потери функциональности?
amyboose, спасибо за наводку.
 
amyboose

amyboose

Client
Регистрация
21.04.2016
Сообщения
2 312
Благодарностей
1 191
Баллы
113
1) зачем делать апкаст и даункаст несколько раз - создал объект, присвоил свойства и добавил в свою коллекцию потом, а не наоборот
2) зачем использовать is и приведение в одном и том же месте, если есть as и проверка на null?
3) автосвойства сокращают код в размерах
 
amyboose

amyboose

Client
Регистрация
21.04.2016
Сообщения
2 312
Благодарностей
1 191
Баллы
113
Для такого случая придумали паттерн "Цепочка обязанностей", чтобы все было красиво
 
dephhs1

dephhs1

Client
Read only
Регистрация
08.01.2017
Сообщения
112
Благодарностей
32
Баллы
28
все тоже что и в сишарпе можно сделать и обычным средством зенки . есть смыслв сишарпе лиш в том что если пишеш на пост гет то он будет работать быстрее чем кубик к кубику и все . но зато понятнее чем СЫСАРП:D
 
Lord_Alfred

Lord_Alfred

Client
Регистрация
09.10.2015
Сообщения
3 916
Благодарностей
3 856
Баллы
113
Moadip

Moadip

Client
Регистрация
26.09.2015
Сообщения
509
Благодарностей
823
Баллы
93
Nikitos704 сказал(а):
Вроде бы цель достигнута. Но все равно как-то громоздко. Есть общий класс (в нем общие переменные для всех дочерних классов). Есть наследники (2 класса), в каждом наследнике есть метод Display1 и в каждом наследнике он выполняет свою функцию.

Следующая часть кода отвечает за создание массива, тип каждого элемента в массиве будет свой (Employee1, Employee2) и т.д. Заполняю поля элементов массива значениями. И последний цикл. В котором перебираем элементы массива и если тип элемента массива соответствует дочернему классу, то выполняем метод Display1() данного дочернего класса.

Мне код кажется громоздким, его можно как-то подсократить (упростить) без потери функциональности?
Нажмите, чтобы раскрыть...
Как уже писал @amyboose единственный норм вариант, это делать абстрактный класс, наследоваться от него, и в каждом классе переопределять метод Display.
Т.е. для каждого класса "действие" будет уникальным.
C#: 
public abstract class Person
{
   public string FirstName { get; set; }
   public string LastName { get; set; }

   public abstract void Display();
}

public class Employee1 : Person
{
   public override void Display()
   {
     Console.WriteLine("1-й наследник");
   }
}
public class Employee2 : Person
{
   public override void Display()
   {
     Console.WriteLine("2-й наследник");
   }
}
И дальше по коду
C#: 
var employeeList = new List<Person>();

employeeList.Add(new Employee1()
{
   FirstName = "Вася",
   LastName = "Пупкин"
});

employeeList.Add(new Employee2()
{
   FirstName = "Иван",
   LastName = "Петрович"
});


foreach (var person in employeeList)
{
   person.Display();
}
Инициализировать св-ва можно не после того как объект добавлен в коллекцию, как здесь
upload_2018-4-18_13-42-0.png
А сразу при создании объекта и добавлении в коллекцию через иницализатор, так убивается сразу два зайца - сокращается кол-во кода и избегаются ошибки.
upload_2018-4-18_13-46-34.png
Вообще бы порекомендовал подобные эксперименты делать в студии(если еще не там:-)) с плагином ReSharper.

amyboose сказал(а):
Для такого случая придумали паттерн "Цепочка обязанностей", чтобы все было красиво
Нажмите, чтобы раскрыть...
В данном случает этот паттерн не применим. От него не будет толку.
Паттерны надо тоже не бездумно пихать, т.к. это "раздувает" код.
Не всегда применение паттернов целесообразно.
 
  • Спасибо
Реакции: Nikitos704
amyboose

amyboose

Client
Регистрация
21.04.2016
Сообщения
2 312
Благодарностей
1 191
Баллы
113
Moadip сказал(а):
В данном случает этот паттерн не применим. От него не будет толку.
Паттерны надо тоже не бездумно пихать, т.к. это "раздувает" код.
Не всегда применение паттернов целесообразно.
Нажмите, чтобы раскрыть...
Код: 
foreach (object cl in myArrayList)
            {
                if (cl is Employee1)
                {
                    Console.WriteLine(((Employee1)cl).id);
                    ((Employee1)cl).Display1();
                }
                if (cl is Employee2)
                {
                    Console.WriteLine(((Employee2)cl).name);
                    ((Employee2)cl).Display1();
                }
            }
Я к разноплановой логике бы его применил, где не просто нужно вызывать 1 и тот же метод, унаследованный от базового класса, а конфигурировать биржи в зависимости от ситуации. Он более гибкий в плане условий работы и потом если появятся доп условия (например, проверка того, работает ли биржа, подходит ли она нам сейчас и т.п.), то особо-то и менять не придется
P.S.: есть 1 минус - куча кода, если бирж много
 
Последнее редактирование:
Moadip

Moadip

Client
Регистрация
26.09.2015
Сообщения
509
Благодарностей
823
Баллы
93
amyboose сказал(а):
Я к разноплановой логике бы его применил, где не просто нужно вызывать 1 и тот же метод, унаследованный от базового класса, а конфигурировать биржи в зависимости от ситуации. Он более гибкий в плане условий работы и потом если появятся доп условия (например, проверка того, работает ли биржа, подходит ли она нам сейчас и т.п.), то особо-то и менять не придется
Нажмите, чтобы раскрыть...
Вся логика по возможности должна быть сокрыта внутри наследника, ну или базового класса.
Из базового кода дергается только какой то метод, для управления.
Другими словами "наружу" торчат только рычаги управления, все остальное внутри.
Не надо двигатель запихивать в салон.:-)

На эту тему можно вести долгие холивары.
Так же как и нет предела совершенства при рефакторинге.:D
 
  • Спасибо
Реакции: Adigen
Adigen

Adigen

Client
Регистрация
28.07.2014
Сообщения
825
Благодарностей
651
Баллы
93
Nikitos704 сказал(а):
Сделал так:


Вроде бы цель достигнута. Но все равно как-то громоздко. Есть общий класс (в нем общие переменные для всех дочерних классов). Есть наследники (2 класса), в каждом наследнике есть метод Display1 и в каждом наследнике он выполняет свою функцию.

Следующая часть кода отвечает за создание массива, тип каждого элемента в массиве будет свой (Employee1, Employee2) и т.д. Заполняю поля элементов массива значениями. И последний цикл. В котором перебираем элементы массива и если тип элемента массива соответствует дочернему классу, то выполняем метод Display1() данного дочернего класса.

Мне код кажется громоздким, его можно как-то подсократить (упростить) без потери функциональности?
amyboose, спасибо за наводку.
Нажмите, чтобы раскрыть...
Вы просто не используете базовый класс по назначению, вам нет надобности приводить типы, достаточно использовать базовый, т.к. методы и свойства у них одинаковые, разница только в обработчиках
C#: 
class Person
{
   public int Id { get; set; }
   public string Name { get; set; }
   public string FirstName { get; set; }
   public string LastName { get; set; }

   public virtual void Display()
   {
      Console.WriteLine($"{FirstName} {LastName}");
   }
}

class Employee1 : Person
{
   public override void Display()
   {
      Console.WriteLine("1-й наследник");
      //Если надо вызывать и базовый обработчик
      base.Display();
   }
}

class Employee2 : Person
{
   public override void Display()
   {
      Console.WriteLine("2-й наследник");
   }
}
C#: 
List<Person> personList = new List<Person>();
personList.Add(new Employee1()
{
   Id = 10,
   Name = "1-й"
});
personList.Add(new Employee2()
{
   Id = 20,
   Name = "2-й"
});

foreach (Person cl in personList)
{
   Console.WriteLine(cl.Id);
   cl.Display();
}
А вообще, я бы вам посоветовал посмотреть в сторону делегатов, т.к. в вашем случае они возможно лучше подойдут,
 
Последнее редактирование:
  • Спасибо
Реакции: Nikitos704
Вам необходимо войти или зарегистрироваться, чтобы здесь отвечать.

Кто просматривает тему: (Всего: 1, Пользователи: 0, Гости: 1)

Верх Низ