Помогите избавиться от запаха

[Беня]

Всем привет!

В некоторой сборке находится интерфейс IItem и его конкретные реализации ItemA, ItemB и т.д.
Эту сборку использует другая сборка, которая работает с интерфейсом IItem примерно таким образом:

GetSummary(IItem item)
{
  if (item is ItemA) GetSummry((ItemA)item);
  if (item is ItemB) GetSummry((ItemB)item);
  ...
}

GetSummary(ItemA item) {...}
GetSummary(ItemB item) {...}
...

По некоторым причинам я не могу перенести GetSummary в IItem чтобы эту батарею заменить полиморфизмом. Будем считать, что я не имею доступа к коду сборки, в которой лежит IItem.
Как мне поступить? Не могу придумать.
Кроме функции GetSummary есть ещё одна, реализованная по такому же принципу.

[Чапаев]

как я понял , есть три метода с такими сигнатурами

GetSummry(IItem  item)

GetSummry(ItemA  item)

GetSummry(ItemB  item)

можно предложить 2 способа

1. простой

перенести методы в два класса SummaryA, SummaryB - наследники базового, реализующие интерфейс с методом GetSummry(IItem  item).

GetSummry(ItemA  item)

GetSummry(ItemB  item)

переименовать GetSummry(IItem  item), в котором осуществляется выбор по типу класса, так как его основная задача - выбрать метод подсчета итога, а не подсчитать итог.

2. написать простую фабрику для выбора объектов класс SummaryFactory со статическим методом Instance

public statuc SummaryFactory 

{

}

пятница, 10 июня 2016 г., 11:14:28 UTC+3 пользователь Беня написал:

[Jewgeni Bakst]

Немного смущает, что в любом случае придётся преобразовывать IItem к конкретному типу либо в методе-фабрике, либо в классе-фабрике. Без кастов никак?

--

---
Вы получили это сообщение, поскольку подписаны на группу "dotnetconf".
Чтобы отменить подписку на эту группу и больше не получать от нее сообщения, отправьте письмо на электронный адрес dotnetconf+...@googlegroups.com.
Чтобы настроить другие параметры, перейдите по ссылке https://groups.google.com/d/optout.

[Чапаев]

первый раз страница сглючила и отправилась  раньше времени

как я понял , есть три метода с такими сигнатурами

GetSummry(IItem  item)

GetSummry(ItemA  item)

GetSummry(ItemB  item)

можно предложить 2 способа

1. простой

перенести методы в два класса SummaryA, SummaryB - наследники базового, реализующие интерфейс с методом GetSummry(IItem  item).

GetSummry(ItemA  item)

GetSummry(ItemB  item)

переименовать GetSummry(IItem  item), в котором осуществляется выбор по типу класса, так как его основная задача - выбрать метод подсчета итога, а не подсчитать итог.

далее написать простую фабрику для выбора объектов класс SummaryFactory со статическим методом Instance

public static SummaryFactory 

{

publci static IItem   Instance()

{

  if (item is ItemA) return new SummaryA();

}

}

идея  в том , что в обработка суммирования  GetSummry вынесена в  классы,  выбор класса осуществляется фабрикой.

2. способ второй

абстрактная фабрика 

переименовать GetSummry(IItem  item) в метод SummrySelector. аргументом в нее передавать ссылку на абстрактную фабрику, которая будет конструировать объекты , осуществляющие вычисление результатов методами GetSummry(IItem item)

то есть объекты для фабрики

interface ISummry

{

GetSummry(IItem  item)

}

реализации 

public class SummryA: ISummry

{

GetSummry(IItem  item)

{

}

}

заводите абстрактный класс

public abstract static SummaryFactory 

{

public static SummaryFactory  Instance(IItem item)

{

if (item is ItemA)

 return new SummaryFactoryA(item) ;

}

public abstaract ISummry SummryCreate();

 

}

конкретную реализацию

public SummaryFactoryA: SummaryFactory

{

public ISummry SummryCreate()

{

return new SummryA();

}

}

далее вы имеете метод 

public SummrySelector(SummaryFactory factory, IItem item)

{

var summary=factory.SummryCreate();

summary.GetSummry(item);

}

ну и перед вызовом SummrySelector нужно сконструировать фабрику 

var  factory=SummrySelector.Instance(item);

минус подхода - придется добавлять нужные фабрики, 
плюс - все в одном месте, SummrySelector отпадет зависимость от ItemA и будет сравнительно короткий.

[Jewgeni Bakst]

То что вы написали в первом способе не будет работать:

* Класс SummaryA, реализующий интерфейс с методом GetSummary(IItem  item) не может переопределить тип параметра в GetSummary(ItemA  item)
* Метод SummaryFactory.Instance() должен возвращать интерфейс с методом GetSummary, а не IItem

[Николай Мельков]

А если использовать Generic method 

<T>GetSummary(T item) where T : Stream{

}

пятница, 10 июня 2016 г., 13:14:28 UTC+5 пользователь Беня написал:

[Беня]

В способе про абстрактную фабрику я не вижу где находятся конкретные реализации IItem. Кто в конечном итоге принимает ItemA?

[Беня]

Николай, я не понял вашу мысль. Можете пояснить более подробным примером?

[Чапаев]

так конкретные реализации в 

public class SummryA: ISummry

{

GetSummry(IItem  item)

{

}

}

ведь ItemA наследник  IItem  поэтому можно написать так

var item= new ItemA();

var sum=new SummryA();

sum.GetSummry(item);

пятница, 10 июня 2016 г., 14:32:30 UTC+3 пользователь Николай Мельков написал:

[Чапаев]

пишу без подсказок среды - могу делать ошибки.

а так метод конечно должен быть таким 

public static ISummary  Instance()

{

  if (item is ItemA) return new SummaryA();

}

Summary - интерфейс с соответствующей реализацией GetSummary.

пятница, 10 июня 2016 г., 13:54:21 UTC+3 пользователь Беня написал:

[Jewgeni Bakst]

Для вычисления Summary для каждого конкретного типа нужно обращаться к свойствам этого конкретного типа. Их нет в интерфейсе IItem. Они у всех реализаций разные. К сожалению я не достаточно чётко обозначил это в изначальном вопросе.

--

[Михаил Корчак]

Добрый день. Я так понимаю, что ручного кастинга в данном случае не избежать. Он будет присутствовать так или иначе. Единственное что можно сделать, так это выбрать подходящее место, чтобы спрятать это все дело. Фабрика или билдер в самый раз.

[Denis Kodua]

я конечно ещё только учусь хорошо программить :), потому отнеситесь с пониманием, 

но зачем создавать классы Summary* 

а если для каждого класса Item создать наследника и расширить его?

public class ExtendItem : ItemA, IItem

{

  public string GetSummary()  

  {

     return "summary A";

  }

}

Автор как раз автор пишет, что не может добавить GetSummary в исходные классы,

так создать наследника и расширить поведение класса...

если нельзя наследоваться, ну как вариант использовать фасад

public class ExtendItem : IItem

{

  public ItemA ItemA {get, set}

 

  public string GetSummary()  

  {

     return "summary A";

  }

}

не вариант? 

10 июня 2016 г., 17:15 пользователь Михаил Корчак <korc...@gmail.com> написал:

[Alew]

Разве замена проверки типа на набор дополнительных абстракций это хорошее решение?

Для начала надо идентифицировать проблему, иначе сложно искать решение.
В данном случае проблемой является нарушение инкапсуляции, и как многие понимают на уровне подсознания вызов должен выглядеть так:
item.GetSummary()
и если отбросить всякие изотерические варианты, то останется два решения:
1. Оставить как есть. Ну может быть перенести проверку типа в метод расширения, благо c# позволяет. Плюсы решения - просто, даже примитивно. Минусы - вы не завоюете всеобщее признание у любителей все переусложнять.
2. Не тащить типы, к коду которых у вас нет доступа в доменную модель, а конвертировать в правильный доменный объект. Плюсы - полный контроль над доменным объектом и как следствие канонический ООП. Минусы - станет сложнее т.к. появится дополнительный слой (anti-corruption layer по Эвансу)
Выбор между этими двумя вариантами зависит исключительно от того как написан проект (если это коммерческий проект) Выделялась ли доменная модель или все написано в стиле big ball of mud? Во втором случае надо делать вариант 1, чтобы не увеличивать энтропию проекта. И ни в коем случае не пытаться постигать DDD на коммерческих проектах, т.к. ничего хорошего их этого не выйдет, все равно все скатится туда же, но вместо простого говнокода у вас будет сложный.

--

[Alew]

Кстати, можно же использовать возможности DLR

и написать такой код

GetSummary(IItem item)
{
  GetSummry((dynamic)item);
}

[Беня]

Хочу тут ответить на мелкие вопросы пользователям @korchakmv, @denis kodua, @alewmt, а потом отдельным номером обсудить интересную тему, предложенную alewmt

@korchakmv: "Я так понимаю, что ручного кастинга в данном случае не избежать. [...] Единственное что можно сделать, так это выбрать подходящее место, чтобы спрятать это все дело."
- Да, на данный момент я поступил именно так.

Есть интерфейс декоратора
   
 public interface IItemFrontendDecorator
    {
        string GetSummary();
        IItemFrontendDecorator Init(IItem item);
    }
 
Есть базовый типизированный класс, имплементирующий этот интерфейс. Он реализует метод Init, в котором спрятан "ручной кастинг".

    public abstract class BaseFrontendDecorator<T> : IItemFrontendDecorator where T : IItem
    {
        protected T Item { get; private set; }
        public abstract string GetSummary();

        public IItemFrontendDecorator Init(IItem item)
        {
            Item = (T)item;
            return this;
        }
    } 
 
Кроме того есть конкретные реализации для всех типов Item и маленькая фабрика декораторов.
Клиентский код выглядит так:
 var deco = DecoFactory.CreateDeco(item);
 if (deco != null)
 {
  Summary = deco.GetSummary();
 }
 
@denis kodua: "а если для каждого класса Item создать наследника и расширить его?"
- Проблема в том, что весь клиентский код работает с типом IItem. То есть сложность не в том, куда положить код GetSummary, а в том как перейти от типа IItem, который не знает никакого GetSummary к типу, который знает.

@alewmt: "перенести проверку типа в метод расширения", "использовать возможности DLR"
- Так как клиентский код должен плясать от IItem не сработает extension. А из-за того что для построения саммари нужны различные свойства конкретных items ничего не выйдет с dynamic.

[Беня]

> Разве замена проверки типа на набор дополнительных абстракций это хорошее решение?

[...]

> Не тащить типы, к коду которых у вас нет доступа в доменную модель, а конвертировать в правильный доменный объект.

В этой теме, которую я начал, речь идёт о конкретном фронтэнде. Бизнес-логика находится в отдельном слое пониже. GetSummary строит описание объекта, которое будет показано пользователю в некоторой таблице, поэтому этот код лежит в слое приложения, а не в слое бизнес-логики. Это правильное решение?

[Alew]

Дело в том что нет никакого бонуса в том чтобы прятать проверку типа в вашем случае. Вы не получите правильный ООП, потому что нарушение инкапсуляции никуда не денется. У вас есть доменный объект который должен сам отвечать на вопрос о саммари, сам - это ключевое слово. Инкапсуляция это НЕ сокрытие полей класса с выставлением наружу пропертей 1 в 1 с полями, как ошибочно полагает огромное количество разработчиков. Инкапсуляция это компоновка данных и обрабатывающих их методов в одну сущность. Поэтому когда вы лезете в объект чтобы получить саммари вы получаете нарушение инкапсуляции. Вот именно в этом проблема кода.

Еще раз повторю мысль. Необязательно писать в строжайшем соответствии с ООП, а иногда даже противопоказано.

ООП, как бы пародоксально это не звучало, совсем не "наследование инкапсуляция и полиморфизм" это философская концепция, а 3 указанных категории это только инструменты реализации. Поэтому используя набор паттернов, чтобы в конце цепочки получить полиморфный вызов вы не получите ООП, вы получите орден адепта культа карго. Делать правильный ООП надо уметь.

Какой вариант из этих у вас не заработал?

с динамиком:

GetSummary(IItem item)
{
  GetSummry((dynamic)item);
}

с экстеншеном:

GetSummary(this IItem item)

{
  if (item is ItemA) GetSummry((ItemA)item);
  if (item is ItemB) GetSummry((ItemB)item);
}

[Alew]

item.GetSummary() это и есть бизнес логика.

[Jewgeni Bakst]

Никакой не заработал.

В реализации конкретных GetSummary необходимо обращаться к разным свойствам. Duck typing не работает.
ItemA
 PropertyA;
 Property1;
 
ItemB
 PropertyB;
 Property2; 
 
Всё что было общего у них, уже вынесено в интерфейс. Остался разнобой, к которому как раз и обращается GetSummary

Вариант с экстеншн метод не работает потому что на какой тип я буду вешать экстеншн? Клиент знает только интерфейс IItem. Для конкретной реализации GetSummary мне нужен конкретный тип ItemA, ItemB и так далее.

Но это всё уже не важно. Давайте лучше обсудим нарушение инкапсуляции.
Рассмотрим такой пример:
Есть набор уже извеснтых нам айтемов IEnumerable<IItem> который пользователь видит в гриде.
Данное конкретное приложение отображает их в гриде строкой, которую подготавливает GetSummary
Как эта строка должна выглядеть -- зависит от конкретного приложения. Эта логика должна оставаться на уровне приложения.
Как подружить в таком случае инкапсуляцию и распределение ответственности по слоям?

[Alew]

Что не так с этим кодом?

using System;

namespace ConsoleApplication1
{
public interface IItem
{
}

public class ItemA : IItem
{
public string A { get { return "A"; } }
}

public class ItemB : IItem
{
public string B { get { return "B"; } }
}

static class Program
{
private static void Main()
{
for (int i = 0; i < 2; ++i)
{
IItem item = i % 2 == 0 ? (IItem)new ItemA() : new ItemB();

Console.WriteLine(item.GetSummary());
}
Console.ReadLine();
}

static string GetSummary(this IItem item)
{
return "Summary " + Output((dynamic)item);
}

static string Output(ItemA item)
{
return item.A;
}

static string Output(ItemB item)
{
return item.B;

[Denis Kodua]

а создать паршл класс и в нем реализовать GetSummary ? не выйдет?

13 июня 2016 г., 16:51 пользователь Alew <ale...@gmail.com> написал:

--

--- Вы получили это сообщение, поскольку подписаны на группу dotnetconf.

Чтобы отменить подписку на эту группу и больше не получать от нее сообщения, отправьте письмо на электронный адрес dotnetconf+...@googlegroups.com.

Настройки подписки и доставки писем: https://groups.google.com/d/optout.

[Jewgeni Bakst]

Я не знал про этот трюк с кастом в dynamic. Спасибо!

--

--- Вы получили это сообщение, поскольку подписаны на группу dotnetconf.

Чтобы отменить подписку на эту группу и больше не получать от нее сообщения, отправьте письмо на электронный адрес dotnetconf+...@googlegroups.com.

[Alew]

Функциональные требования это бизнес логика, даже если это разные фронтэнды к общему бэкенду. Сама формулировка "данные зависят от приложения" запутывает. Уровень приложения это технический аспект - как организовать взаимодействие пользователя с моделью.

On 13.06.2016 15:17, Jewgeni Bakst wrote:

[Alew]

Придумано для автогенерации и не стоит применять не по назначению.
Экстеншн тоже не очень чистый и не самый очевидный хак, но все уже, вроде как, к ним привыкли.

[Артём Мурадов]

привет. 

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

void Main()

{

var container = new WindsorContainer();

container.Register(Component.For<IProcessor<MyItem1>>().ImplementedBy<Processor1>());

container.Register(Component.For<IProcessor<MyItem2>>().ImplementedBy<Processor2>());

//....

Process(container, new MyItem1());

Process(container, new MyItem2());

}

static void Process(IWindsorContainer container, IItem item)

{

var generic = typeof(IProcessor<>);

var spec = generic.MakeGenericType(item.GetType());

var proc = container.Resolve(spec) as IProcessor;

proc.Proccess(item);

}

public interface IItem

{

}

public class MyItem1 : IItem

{

}

public class MyItem2 : IItem

{

}

public interface IProcessor

{

void Proccess(IItem item);

}

public interface IProcessor<T> : IProcessor where T : IItem

{

void Proccess(T item);

}

public class Processor1 : IProcessor<MyItem1>

{

public void Proccess(MyItem1 item)

{

Console.WriteLine("MyItem1");

}

public void Proccess(IItem item)

{

Proccess(item as MyItem1);

}

}

public class Processor2 : IProcessor<MyItem2>

{

public void Proccess(MyItem2 item)

{

Console.WriteLine("MyItem2");

}

public void Proccess(IItem item)

{

Proccess(item as MyItem2);

    }

}

пятница, 10 июня 2016 г., 10:14:28 UTC+2 пользователь Беня написал: