Pull to refresh
31
0

Программист C++/Scala/Go

Send message
Приятно слышать, давно не смотрел сколько «весит» инсталятор .Net Framework — быстрый интернет разбаловал :) Тогда тем более не понятно, что там такого сделано, хоят может один инсталятор содержит ПО и драйвера для 32bit и 64bit системы.
Скорей всего, программа написана с использованием .Net и инсталятор включает его в себя. Сейчас уже почти нормой стало, когда приложение занимает пару десяток мегабайт, а инсталятор несколько сотен мегабайт.
Согласен, пример показывает ту же идею — как можно избавиться от кучи if — else, которая появитсья при реализации решения «в лоб» используя возможности языка. А остальное уже особенности конкретного языка и предпочтений разработчика.
Заказчику все равно и то, как система спроектирована, и какие паттерны применял или не применял разработчик, и каким методологиям он следовал. Заказчику важно получить продукт в срок, за оговоренные деньги и чтобы этот продукт работал так, как он себе это представляет. Программа может «глючить», могут быть «утечки» памяти, если программа не предназначена работать без перезапуска длительное время, главное, чтобы это не было заметно заказчику. Но разговор не о взаимоотношениях заказчик-исполнитель. Разговор о самосовершенствовании разработчика.

Если программист хочет выполнять задачи эффективно, если ему не все равно, что думают о его коде другие разработчики, то он должен изучать свои инструменты и уметь их правильно использовать. А в отношении качества кода разработчик должен только себе (может быть, своим коллегам, если работает в команде). Иными словами, если человек хочет что-то уметь, он должен этому учиться. А кому он должен? — только себе.
Похоже, каждый из нас останется при своем мнении. У меня не хватает аргументов убедить Вас, что StaticDispatcher это не столь сложно как кажется, и в то же время меня не убеждают Ваши аргументы, что лучше оставить все как есть.

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

Увы протекать — это особенность абстракций, но это не значит, что в каждой из них придется обязательно разбираться или описание будет очень большим. В StaticDispatcher придется разбираться тому, кто его будет писать, дальше менять его уже не понадобится. Это будет практически библиотечный класс.

В любом случае, спасибо за дискуссию, для себя я почерпнул некоторые интересные мысли.
Так что через некоторое время выясняется что приложение иногда падает с ошибками при поиске пересечений…

Согласен, был не прав, юнит тест все таки нужен.

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

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

Вариант развития событий 2. Понадобилось программисту вычислить пересечение 2х фигур. Зашёл он в класс фигуры и ничего не увидел.

Можно сделать объявление функции поиска пересечений рядом с базовым классом для фигур принимающую в качестве параметров две ссылки(или указателя) на базовый класс. Тело функции будет содержать вызов метода StaticDispatcher c HatchingExecutor. И достаточно не толстенной документации а строчки с комментарием. И не придется ничего искать. А если функций для работы с фигурами несколько, то их объявление можно собрать в одном месте.

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

И юнит тест падает, потому что в фигура треугольник была добавлена третьей по счету и в ней были реализованы только пересечения с Прямоугольником и Элипсом. А кроме них оказалось, что есть и другие фигуры. Дальше пограммист должен посмотреть на все оставшиеся классы (ну не на все, еще на пары другой было бы достаточно), что бы понять, что для каждой новой фигуры необходимо реализовать ее пересечение со всеми остальными. и тратит кучу времени на написание однотипных if-else. Чтобы таких изысканий не было стоит сразу, в базовом классе, написать комментарий к виртуальному методу поиска пересечения/штриховки.

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


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

В реальных проектах считается хорошим тоном запускать все тесты перед отправкой кода в репозиторий.

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

Можно и без помощи IDE найти два файлика. В любом случае просмотреть 2 маленьких файлика проще, чем один гигантский в 100 раз больше обоих.

Откуда в 100 раз больше обоих — посмотреть заголовочный файл, в котором есть только объявление методов этого достаточно. На 10 фигурах это будет 55 методов, в последней фигуре будет 10 методов проверки пересечений + собственные методы (перегруженные из базового класса, сеттеры/геттеры...). Так что не в 100 раз больше. Но это вопрос вкуса, и то какие манипуляции в целом производятся с фигурами.

Вариант, который вы предлагаете, и с подсветкой, и без подсветки воспринимается тяжело.

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

вопрос был что в конкретно данном случае вам не нравится, а не почему в целом вам не нравятся проверки типов

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

А в чем тогда смысл следовать KISS, если все проверки можно свалить на юнит тесты? Запуск юнит теста, только для проверки, что все варианты предусмотрели — дольше, чем просто пробежаться взглядом по десятку функций. Юнит тест интереснее использовать, для проверки правильности расчета пересечений, что без вникания в алгоритм сделать сложно, и все равно можно упустить какие-нибудь пограничные варианты.

Мое решение так же покрывается юнит тестом. Для варианта сборки юнит теста можно предусмотреть, чтобы тип каждой фигуры регистрировался в контейнере, а в юнит тесте был бы проход по всем элементам этого контейнера. Регистрацию можно делать при помощи макроса в который передается новый тип. Макрос будет раскрываться либо в ничего, если это сбора продукта или в регистрацию типа, если это сборка юнит теста. Контейнер, может хранить указатели на функции, создающие нужные фигуры и возвращающие указатель на базовый класс. Таким образом код юнит теста не меняется при добавлении нового типа фигур.

Почти во всех современных IDE есть горячие клавиши для быстрого перехода к нужному классу.

Как-то давно я слышал высказывание, что если код тяжело читается в простом текстовом редакторе без подсветки синтаксиса, то это плохой стиль кодирования. То же можно сказать и об организации кода. Если навигация по коду не удобна без средств предоставляемых IDE, стоит задуматься, а все ли хорошо сделано. Эта проблема актуальна и при кодревью, не всегда при кодревью можно воспользоваться возможностями IDE для навигации по коду.

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

а это опять же проверки через динамическое приведение типов

Я как раз и имел ввиду, что от них никуда не деться, а они и есть самое запутанное и опасное место, в котором можно допустить ошибки и внесение изменений в него хотелось бы минимизировать. В вашем варианте каждый новый класс будет сложнее предыдущих по количеству таких проверок.
Если реализовывать расчет пересечения как виртуальный метод класса Shape, все равно придется искать с какой именно фигурой мы пересекаемся, а это опять же проверки через динамическое приведение типов.

Если Прямоугольник знает как пересекаться с Элипсом, то в Элипсе уже это вроде можно и не реализовывать. А при добавлении новой фигуры как быть? Реализовывать возможность пересечения с остальными фигурами только в ней, а в ранее реализованные классы не трогать? Выглядит логично. В итоге получается путаница что один класс умеет пересекаться только с самим собой, другой с самим собой и еще какими-то классами, все это разбросанно по разным файлам и со временем может превратиться в кошмар. Ну и фигуры должны иметь информацию о других фигурах внутри себя. В итоге сложность в визуальном контроле, что для всех фигур предусмотрели расчет пресечения.

Внешняя функция расчета пересечения позволяет фигурам не зависеть друг от друга, все методы расчета пересечений собраны в одном месте, а это упрощает визуальный контроль, что ничего не забыли.

Ваше решение проще реализовать, в нем проще разобраться, но поддерживать его сложнее.

В варианте из Александреску — сложнее разобраться, но дальнейшее добавление фигур сводится к описанию класса новой фигуры и добавлению методов расчета пересечений, список типов можно передавать не отдельно, а вместе с классом ДелательПересечений, что-то типа:
class ДелательПересечений
{
public:
    typedef TYPELIST_3<Прямоугольник, Элипс, Многоугольник> ПоддерживаемыеФигуры.
}


И опять же можно написать юнит тест.

в том числе благодаря тому что имена переменных (p1, p2, lhs) не информативны

Во-первых их можно переименовать (выбор названия для переменных это вообще отдельная тема), а во-вторых в чем не информативность — функция поиска пересечения двух фигур принимает параметры lhs, rhs — левый фигура, правая фигура. Можно просто shape1 и shape2. При таком использовании и учитывая что перемененных мало (lhs, rhs, p1, p2) понять их значение не сложно.

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

Нам необходимо рисовать различные фигуры. Все фигуры
унаследованы от базового класса Shape и хранятся как указатели на него.
Например в векторе указателей на Shape, как именно они хранятся не столь важно,
важно, что имеем мы указатель на Shape, который за собой скрывает разные фигуры.

Скрытый текст
При пересечении фигур необходимо заштриховать область их пересечения.
Так как разработать универсальный алгоритм штриховки для разных сочетаний
фигур (прямоугольник-прямоугольник, прямоугольник-многоугольник,
многоугольник-многоугольник, эллипс-многоугольник, эллипс-эллипс) довольно
сложно и он скорей всего будет не очень эффективным, то реализуем для каждого
вариант свой алгоритм.

void DoHatchArea(Rectangle&, Rectangle&);
void DoHatchArea(Rectangle&, Ellipse&);
void DoHatchArea(Rectangle&, Poly&);
void DoHatchArea(Ellipse&, Poly&);
void DoHatchArea(Ellipse&, Ellipse&);
void DoHatchArea(Poly&, Poly&);


Отлично, алгоритмы штриховки есть, осталось вызвать нужный для каждой пары фигур.
В C++ есть dynamic_cast, который позволяет привести указатель базовый класс к
дочернему, если он указывает на дочерний класс. Простая реализация «в лоб» будет
выглядеть как-то так:

void DoubleDispatch(Shape& lhs, Shape& rhs)
{
    if(Rectangle* p1 = dynamic_cast<Rectangle*>(&lhs))
    {
        if(Rectangle* p2 = dynamic_cast<Rectangle*>(&rhs))
            DoHatchArea(*p1, *p2);
        else if(Ellipse* p2 = dynamic_cast<Ellipse*>(&rhs))
            DoHatchArea(*p1, *p2);
        else if(Poly* p2 = dynamic_cast<Poly*>(&rhs))
            DoHatchArea(*p1, *p2);
        else
            Error("Неопределенное пересечение");
    }
    else if(Ellipse* p1 = dynamic_cast<Ellipse*>(&lhs))
    {
        if(Rectangle* p2 = dynamic_cast<Rectangle*>(&rhs))
            DoHatchArea(*p2, *p1);
        else if(Ellipse* p2 = dynamic_cast<Ellipse*>(&rhs))
            DoHatchArea(*p1, *p2);
        else if(Poly* p2 = dynamic_cast<Poly*>(&rhs))
            DoHatchArea(*p1, *p2);
        else
            Error("Неопределенное пересечение");
    }
    else if(Poly* p1 = dynamic_cast<Poly*>(&lhs))
    {
        if(Rectangle* p2 = dynamic_cast<Rectangle*>(&rhs))
            DoHatchArea(*p2, *p1);
        else if(Ellipse* p2 = dynamic_cast<Ellipse*>(&rhs))
            DoHatchArea(*p2, *p1);
        else if(Poly* p2 = dynamic_cast<Poly*>(&rhs))
            DoHatchArea(*p1, *p2);
        else
            Error("Неопределенное пересечение");
    }
    else
    {
        Error("Неопределенное пересечение");
    }
}


Получилось все довольно просто и понятно, но что будет, когда возможных фигур
будет больше? Программа превратится в нагромождение if-else и модифицировать
ее будет крайне сложно.

Что же делать? На помощь приходят шаблоны. Реализуем класс, который будет
содержать алгоритмы штриховки и обработку ошибки, если пересечение
неопределенно:
class HatchingExecutor
{
public:
    //Разные алгоритмы штризовки области пересечения
    void DoHatchArea(Rectangle&, Rectangle&);
    void DoHatchArea(Rectangle&, Ellipse&);
    void DoHatchArea(Rectangle&, Poly&);
    void DoHatchArea(Ellipse&, Poly&);
    void DoHatchArea(Ellipse&, Ellipse&);
    void DoHatchArea(Poly&, Poly&);

    //Функция для обработки ошибок
    void OnError(Shape&, Shape&);
};


И теперь перепишем поиск:
template<
    class Executor,
    class BaseLhs,
    class TypesLhs,
    class BaseRhs = BaseLhs,
    class TypesRhs = TypesLhs,
    typename ResulType = void
>
class StaticDispatcher
{
    typedef typename TypesLhs::Head Head;
    typedef typename TypesLhs::Tail Tail;
public:
    static ResultType Go(BaseLhs& lhs, BaseRhs& rhs, Executor& exec)
    {
        if(Head* p1 = dynamic_cast<Head*>(&lhs))
        {
            return StaticDispatcher<
                Executor, 
                BaseLhs, NullType, 
                BaseRhs, TypesRhs>::DispatchRhs(*p1, rhs, exec);
        }
        else
        {
            return StaticDispatcher<
                Executor,
                BaseLhs, Tail,
                BaseRhs, TypesRhs>::Go(lhs, rhs, exec);
        }
    }

    template<class SomeLhs>
    static ResultType DispatchRhs(SomeLhs& lhs, BaseRhs& lhs, Executor& exec)
    {
        typedef typename TypesRhs::Head Head;
        typedef typename TypesRhs::Tail Tail;

        if(Head* p2 = dynamic_cast<Head*>(&rhs))
        {
            return exec.Fire(lhs, *p2);
        }
        else
        {
            return StaticDispatcher<
                Executor,
                SomeLhs, NullType,
                BaseRhs, Tail>::DispatchRhs(lhs, rhs, exec);
        }
    }
}

template<
    class Executor,
    class BaseLhs,
    class BaseRhs,
    class TypesRhs,
    typename ResultType
>
class StaticDispatcher<
    Executor,
    BaseLhs, NullType,
    BaseRhs, TypesRhs,
    ResulType>
{
    static void Go(BaseLhs& lhs, BaseRhs& rhs, Executor& exec)
    {
        exec.OnError(lhs, rhs);
    }
}

template<
    class Executor,
    class BaseLhs,
    class TypesLhs
    class BaseRhs,
    class TypesRhs,
    typename ResultType
>
class StaticDispatcher<
    Executor,
    BaseLhs, TypesLhs,
    BaseRhs, NullType,
    ResulType>
{
    static void DispatchRhs(BaseLhs& lhs, BaseRhs& rhs, Executor& exec)
    {
        exec.OnError(lhs, rhs);
    }
}



А тепрь как это все использовать:
typedef StaticDispatcher<
    HatchingExecutor, Shape,
    TYPELIST_3(Rectangle, Ellipse, Poly)> Dispatcher;
Shape* p1 = ...;
Shape* p2 = ...;
HatchingExecutor exec;
Dispatcher::Go(*p1, *p2, exec);


Теперь в при добавлении новой фигуры достаточно заменить
TYPELIST_3(Rectangle, Ellipse, Poly) на TYPELIST_4(RoundedRectangle, Rectangle, Ellipse, Poly) и добавить методы в HatchingExecutor.

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


Пример взят из книги Андрея Александреску «Современное проектирование на C++».
Это если читатель знает про паттерны. А если нет? Необходимость искать что-то в процессе чтения статьи не доставляет приятных эмоций. Статья хороша для начинающих, так как принцип KISS на слуху и их статья может заинтересовать. А парочка примеров, может даже на разных языках, добавят красок статье. Она станет более реальной более практичной более близкой читателю.
Поддерживаю предыдущих читателей. Наличие примеров помогает быстрее понять суть, а начинающие могут быть не знакомы с книгами в которых рассказывается про рефакторинг и патерны проектирования, да и не иметь достаточного опыта, чтобы выявить примеры из реального кода. Так что есть смысл привести парочку примеров. Иначе статья оставляет впечатление не завершенности.

Первое что пришло в голову, но что можно встретить в реальных проектах. Есть структура, в том числе хранит строки заранее известной максимальной длинны. Можно использовать массив символов, а можно std::string:
struct S1
{
//прочие поля структуры...
char name[50];
};

struct S2
{
//прочие поля структуры...
std::string name;
};


Без дополнительной информации не очень ясно какой из вариантов соответствует KISS, а какой его нарушает.
Вы правы, спасибо, исправил.
В проекте у базовых структур нет конструкторов по-умолчанию, а boost::mpl::inherit требует наличия конструктора по-умолчанию, так что boost::mpl::inherit не использовал, а в стать вполне можно.
Спасибо за замечание.
В C++11 добавили списки инициализации (std::initializer_list), я узнал об этом из небольшой заметки, в которой сравнивался C++03 и C++11. К статье приложена pdf со слайдами, описание начинается с 32 стр. К сожалению, подробно описать не могу, так как сам только-только начал погружаться в мир C++11 :)

Можно записать еще проще:
setAxisSize( {1024,1024} );
Если бы вместо std::vector был бы QVector из Qt, все выглядело бы гораздо симпатичнее.
setAxisSize(QVector()<<1024<<1024);

В C++11 можно записать так:
setAxisSize( std::vector<int>() = {1024,1024} );

Пока я писал код и статью, мне это казалось прозрачным :). Но ваш вопрос заставил сомневаться и пересмотреть код еще раз. Пожалуй в указанном месте можно вообще было обойтись без указателей:

selector 
Executor<T, sizeof( selector( v, v ) )>()( v );


Но Ваш вариант, действительно, гораздо проще. Увлекся я что-то с шаблонами не к месту :)
Прошу прощения, забыл указать вариант когда нет наследования ни от HasMainId0, HasMainId1, HasAnotherId, тогда возвращается для обоих 0xFFFFFFFF.

Да, такой вариант тоже возможен. Единственное, что так как есть еще общий интерфейс в шаблоне для DynamicData нужно будет его реализовать и вызывать из него уже getIds реализованый в DataM. А для случая который я забыл указать можно добавить наследование от структуры.

struct NoIds
{
  virtual void getIds(unsigned& id0, unsigned& id1)
  {
    id0 = 0xFFFFFFFF; id1=0xFFFFFFFF;
  }
}


Но такой вариант мне кажется менее красивым.

Information

Rating
Does not participate
Location
Санкт-Петербург, Санкт-Петербург и область, Россия
Registered
Activity