Сложный код требует читабельности и комментариев

Andrey Khatimlianskii 2021.02.01 16:54 #101

Georgiy Merts:

"Апофеозом" считаю одно выражение fxsaber'a, про которое он сам не смог сказать, как оно работает, заявив просто, что "код многократно оттестирован, и работает". Такого, на мой взгляд, быть не должно:

Этот код проверяет возможность исполнения ордера otfFilingType, и возвращает его, если он доступен на символе strSymbol, иначе - корректный вариант.

Я совершенно не могу понять, как он работает. И только полагаюсь на авторитет fxsaber'a.

Может быть, кто-нибудь объяснит ?

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

Andrei Trukhanovich 2021.02.01 17:04 #102

Igor Makanu:

Ваш пример из "цель оправдывает средства"

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

А такой рисунок видели? Обсуждение статьи "Ядерная оценка Никогда не изучал программирование.

Igor Makanu 2021.02.01 17:27 #103

Andrei Trukhanovich:

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

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

ЗЫ: высока вероятность, что первоначальный вариант маленького монстра выглядел более наглядно ;)

Вопрос к профессиональным трейдерам Машинное обучение в трейдинге: Вопрос к администрации сайта

fxsaber 2021.02.01 17:43 #104

Igor Makanu:

ЗЫ: высока вероятность, что первоначальный вариант маленького монстра выглядел более наглядно ;)

Взял из ZIP (содержит самый первый релиз).

  static bool VirtualOrderSelect( const TICKET_TYPE Index, const int Select, const int Pool = MODE_TRADES )
  {
    return(VIRTUAL::SelectOrders ? VIRTUAL::SelectOrders.OrderSelect(Index, Select, Pool) :
           #ifdef __MQL5__
             #ifdef __MT4ORDERS__
               ::OrderSelect(Index, Select, Pool)
             #else // __MT4ORDERS__
               false
             #endif // __MT4ORDERS__
           #else // __MQL5__
             ::OrderSelect(Index, Select, Pool)
           #endif // __MQL5__
           );
  }

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

Renat Fatkhullin 2021.02.01 19:23 #105

Vasiliy Sokolov:

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

Абсолютно верно.

Если говорить про MQL5, то можно забыть про методы "оптимизаций" 10-20-30 летней давности. Писать надо максимально читабельный код. Именно он является мерилом качества, а не хакерские навороты и чистые понты.

Почему?

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

MQL5 компилятору смешно от человеческих попыток наворотами сделать +2% скорости.

Кому интересно, зацените как 8 квадратных корней в разных выражениях были переупорядочены и вычислены за 4 ассемблерные команды.

Форум по трейдингу, автоматическим торговым системам и тестированию торговых стратегий

Ошибки, баги, вопросы

Renat Fatkhullin, 2018.03.13 22:59

Проверка показала, что:

SQRT маппятся в прямые CPU инструкции

SQRT + математические вычисления идут без ветвлений и за одну команду (128 бит данные) вычисляется сразу два корня

Вот этот код превращается в следующий ассемблерный SSE код:

         D1=sqrt((X1-X)*(X1-X)+(Y1-Y)*(Y1-Y));
         D2=sqrt((X2-X)*(X2-X)+(Y2-Y)*(Y2-Y));
         D3=sqrt((X3-X)*(X3-X)+(Y3-Y)*(Y3-Y));
         D4=sqrt((X4-X)*(X4-X)+(Y4-Y)*(Y4-Y));
         D5=sqrt((X5-X)*(X5-X)+(Y5-Y)*(Y5-Y));
         D6=sqrt((X6-X)*(X6-X)+(Y6-Y)*(Y6-Y));
         D7=sqrt((X7-X)*(X7-X)+(Y7-Y)*(Y7-Y));
         D8=sqrt((X8-X)*(X8-X)+(Y8-Y)*(Y8-Y));

        ...
        sqrtsd  xmm1, xmm1
        unpcklpd        xmm4, xmm4
        movapd  xmm3, xmmword ptr [rsp + 432]
        unpcklpd        xmm3, xmmword ptr [rsp + 384]
        subpd   xmm3, xmm4
        mulpd   xmm3, xmm3
        unpcklpd        xmm0, xmm0
        movapd  xmm5, xmmword ptr [rsp + 416]
        unpcklpd        xmm5, xmmword ptr [rsp + 400]
        subpd   xmm5, xmm0
        mulpd   xmm5, xmm5
        addpd   xmm5, xmm3
        sqrtpd  xmm8, xmm5
        movapd  xmm5, xmmword ptr [rsp + 464]
        subpd   xmm5, xmm4
        mulpd   xmm5, xmm5
        movapd  xmm7, xmm9
        subpd   xmm7, xmm0
        mulpd   xmm7, xmm7
        addpd   xmm7, xmm5
        movapd  xmm6, xmm10
        unpcklpd        xmm6, xmm11
        subpd   xmm6, xmm4
        movapd  xmm3, xmmword ptr [rsp + 368]
        unpcklpd        xmm3, xmmword ptr [rsp + 352]
        subpd   xmm3, xmm0
        movapd  xmm4, xmm8
        shufpd  xmm4, xmm4, 1
        sqrtpd  xmm5, xmm7
        mulpd   xmm6, xmm6
        mulpd   xmm3, xmm3
        addpd   xmm3, xmm6
        sqrtpd  xmm15, xmm3
        movapd  xmm0, xmm14
        unpcklpd        xmm0, xmmword ptr [rsp + 336]
        subpd   xmm0, xmm2
        mulpd   xmm0, xmm0
        movapd  xmm2, xmm0
        shufpd  xmm2, xmm2, 1
        addsd   xmm2, xmm0
        movapd  xmm0, xmm15
        shufpd  xmm0, xmm0, 1
        sqrtsd  xmm12, xmm2

Это произведение исскуства вообще-то. 8 корней вычислено за 4 вызова ассемблерной команды. Два double числа вычислялись за один вызов.

При операциях через массив все идет штатно, с проверками, ветвлениями и потерями на конвертации double -> integer index
При работе с массивами в этом примере идет постоянное смешение FPU/ALU, что очень плохо сказывается на производлительности
Оптимизация доступа к динамическому массиву отличная, выше похвал. Но смешение FPU/ALU операций + перевод double -> integer + ветвления тратят время

Общий вывод: математика в MQL5 побеждает за счет идеальной оптимизации. Тут не массивы проигрывают, а математика выигрывает.

Ошибки, баги, вопросы Canvas - это круто! Особенности языка mql4, тонкости

Aleksey Mavrin 2021.02.01 19:29 #106

Georgiy Merts:

С чего это ?

Как раз наоборот, с двумя "ифами" - куда проще, чем с оператором "или".

Явно проще проследить сперва одно условие, и уйти из функции, в случае выполнения, а потом проверить другое условие, и тоже уйти, в случае выполнения. Чем прикидывать, что получается в результате сложного условия через логическое "или" (которое запросто можно спутать и "и"), и следить за обоими вариантами возврата.

Довольно смешно читать ниже, что "оправдание такому - отладка", ведь это и значит, что такой код куда более понятен (иначе зачем он в отладке ?).

"Апофеозом" считаю одно выражение fxsaber'a, про которое он сам не смог сказать, как оно работает, заявив просто, что "код многократно оттестирован, и работает". Такого, на мой взгляд, быть не должно:

Этот код проверяет возможность исполнения ордера otfFilingType, и возвращает его, если он доступен на символе strSymbol, иначе - корректный вариант.

Я совершенно не могу понять, как он работает. И только полагаюсь на авторитет fxsaber'a.

Может быть, кто-нибудь объяснит ?

Я когда-то сел и разобрал поэтапно, кажется понадобилась ручка с бумажкой)

Для чего этот разбор пригодился - я понял что в случае смены структуры перечислений всё сломается) т.к. используются целочисленные отношения значений перечислений (которые по самой идее перечислений как раз должны быть инкапсулированы). Стараюсь сам такого избегать, максимум отношения больше-меньше. Для имеющих дело с WinAPI наверное привычно.

Советники: МTC Сombo Юрик Вопрос - уточнение по

Georgiy Merts 2021.02.02 06:03 #107

Andrey Khatimlianskii:

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

Да, я так и попытался. Но, не хватило мотивации разбирать...

Georgiy Merts 2021.02.02 06:14 #108

Renat Fatkhullin:

Абсолютно верно.

Если говорить про MQL5, то можно забыть про методы "оптимизаций" 10-20-30 летней давности. Писать надо максимально читабельный код. Именно он является мерилом качества, а не хакерские навороты и чистые понты.

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

Там, где "прозрачно" сделать не получается - обязательны подробные комментарии, почему именно так, а не иначе.

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

Новая версия платформы MetaTrader Любые вопросы новичков по ГЛЮК?! Обработка событий нажатия

Vasiliy Sokolov 2021.02.02 12:06 #109

fxsaber:

Код очень простой и короткий (описание). Если его напишите на ФП, будет интересно сравнить.

Пожалуйста. C# в ФП стиле:

using System;
using System.Linq;
using System.Collections.Generic;
using static FeesCalculator.FeesCalcs;
using System.Text;
using static System.Math;

namespace FeesCalculator
{
    public static class EnumerableExt
    {
        /// <summary>
        /// Call of function 'action' for each element if sequence (mapping).
        /// </summary>
        /// <typeparam name="T"></typeparam>
        /// <param name="seq">Enumerable sequence</param>
        /// <param name="action">Need action</param>
        public static void Map<T>(this IEnumerable<T> seq, Action<T> action)
        {
            foreach (var item in seq)
                action(item);
        }
    }
    /// <summary>
    /// Инвестиционный результат
    /// </summary>
    public record AccountRecord
    {
        public double Investor { get; init; }
        public double Manager { get; init; }
        public double Summary { get; init; }

        public static AccountRecord operator *(AccountRecord r, double v)
        {
            return new AccountRecord
            {
                Investor = r.Investor * v,
                Manager = r.Manager * v,
                Summary = r.Summary * v
            };
        }
        public override string ToString()
        {
            StringBuilder sb = new StringBuilder();
            sb.Append(nameof(Investor)).Append(" = ").Append(Investor.ToString("F4")).Append(' ');
            sb.Append(nameof(Manager)).Append(" = ").Append(Manager.ToString("F4")).Append(' ');
            sb.Append(nameof(Summary)).Append(" = ").Append(Summary.ToString("F4"));
            return sb.ToString();
        }
    }
    /// <summary>
    /// Параметры оферты
    /// </summary>
    public record PammSet
    {
        /// <summary>
        /// Доходность за расчетный период
        /// </summary>
        public double Perfomance { get; init; }
        /// <summary>
        /// Вознаграждение управляющего
        /// </summary>
        public double Gift { get; init; }
        /// <summary>
        /// Количество расчетных периодов
        /// </summary>
        public int N { get; init; }
        /// <inheritdoc/>
        public override string ToString()
        {
            StringBuilder str = new StringBuilder();
            str.Append("Доходность за расчетный период: ").Append((Perfomance * 100.0).ToString("F1")).Append("%\t\n");
            str.Append("Вознаграждение управляющего: ").Append((Gift * 100.0).ToString("F1")).Append("%\t\n");
            str.Append("Количество расчетных периодов: ").Append(N);
            return str.ToString();
        }

    }
    /// <summary>
    /// Формулы расчета
    /// </summary>
    public class FeesCalcs
    {
        /// <summary>
        /// Если управляющий снимает деньги в конце каждого расчетного периода
        /// </summary>
        /// <param name="Performance"></param>
        /// <param name="Gift"></param>
        /// <param name="N"></param>
        /// <returns></returns>
        public static AccountRecord Set1(PammSet set)
        {
            Func<double> investor = () => Pow(1 + (set.Perfomance) * (1 - set.Gift), set.N);
            Func<double> manager = () => (investor() - 1) * set.Gift / (1 - set.Gift);
            AccountRecord ac = new AccountRecord
            {
                Investor = investor(),
                Manager = manager(),
                Summary = investor() + manager()
            };
            return ac;
        }
        /// <summary>
        /// Если ничего не делалось в расчетные периоды
        /// </summary>
        /// <param name="Performance"></param>
        /// <param name="Gift"></param>
        /// <param name="N"></param>
        /// <returns></returns>
        public static AccountRecord Set2(PammSet set)
        {
            Func<double> summary = () => Pow(set.Perfomance + 1, set.N);
            Func<double> manager = () => (summary() - 1) * set.Gift;
            double s = summary();
            double d = manager();
            AccountRecord ac = new AccountRecord
            {
                Summary = summary(),
                Manager = manager(),
                Investor = summary() - manager()
            };
            return ac;
        }
        /// <summary>
        /// Если управляющий снимает деньги в конце каждого расчетного периода и реинвестирует их
        /// </summary>
        /// <param name="Performance"></param>
        /// <param name="Gift"></param>
        /// <param name="N"></param>
        /// <returns></returns>
        public static AccountRecord Set3(PammSet set)
        {
            Func<double> summary = () => Pow(set.Perfomance + 1, set.N);
            Func<double> investor = () => Pow(1 + (set.Perfomance) * (1 - set.Gift), set.N);
            return new AccountRecord
            {
                Summary = summary(),
                Investor = investor(),
                Manager = summary() - investor()
            };
        }
        /// <summary>
        /// Сопостовление: описание - функция расчета
        /// </summary>
        public static readonly Dictionary<string, Func<PammSet, AccountRecord>> DescrToAccountRecord = new Dictionary<string, Func<PammSet, AccountRecord>>
        {
            {"Если управляющий снимает деньги в конце каждого расчетного периода (1)", Set1 },
            {"Если ничего не делалось в расчетные периоды (2)", Set2 },
            {"Если управляющий снимает деньги в конце каждого расчетного периода и реинвестирует их (3)", Set3 },
        };
    }
    class Program
    {
        static void Main(string[] args)
        {
            var _ = args.Select((a) => double.Parse(a)).Take(3).ToList();
            PammSet pamm = new PammSet
            {
                Perfomance = _[0]/100.0,
                Gift = _[1]/100.0,
                N = (int)_[2]
            };
            Console.WriteLine($"Данные для инвестиционного счета со следующими характеристиками:\n\n{pamm}\n");
            Console.WriteLine("Конечный результат для Инвестора, Управляющего и Суммарно:\n");
            Func<KeyValuePair<string, Func<PammSet, AccountRecord>>, string> f = (kvp) => kvp.Key + ".\n" + kvp.Value(pamm).ToString() + "\n";
            Enumerable.Select(DescrToAccountRecord, f).Map((s) => Console.WriteLine(s));
            Console.WriteLine(pamm);
            Func<int, PammSet> toPamm = (n) => new PammSet { Perfomance = pamm.Perfomance, Gift = pamm.Gift, N = n };
            Func<PammSet, IEnumerable<AccountRecord>> toAccs = (n) => DescrToAccountRecord.Select((s) => s.Value(n));
            var accounts = Enumerable.Repeat(1, pamm.N).Select((a, b) => a + b).Select(toPamm).Select(toAccs);
            Func<AccountRecord, string> toString = (ar) => ar.Investor.ToString("F2") + "\t" +
                                                           ar.Manager.ToString("F2") + "\t" +
                                                           ar.Summary.ToString("F2") + "\t";
            int period = default;
            accounts.Map
            (
                (en) => 
                {
                    Console.Write($"{++period}:\t");
                    en.Map((ar) =>
                        Console.Write(toString(ar)));
                    Console.WriteLine();
                }
            );
        }
    }
}

ФП весьма кривоват конечно (так как писал кривой кодер ФПешник, на неполноценном ФП языке C#).Но цель показать что на современных ООП ЯПах можно и в ФП. Ограничено конечно, это не F# или Haskell, но писать в ФП стиле никто не запрещает. Использовать иммутабельность, функции высшего порядка, замыкания, сопоставления и т.д. - все можно, пожалуйста. Только код от этого не становится идеальным почему-то.

з.ы. Общая структура кода намерено мимикрирует под оригинал. Хотя по-хорошему в ФП нужно совсем уж по-другому, без сложных объектов и foreach мимикрирующего под Map, но художник рисовал как мог.

Вопросы по ООП в Асинхронное и многопоточное программирование В догонку

fxsaber 2021.02.02 14:01 #110

Vasiliy Sokolov:

Пожалуйста. C# в ФП стиле:

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

Форум по трейдингу, автоматическим торговым системам и тестированию торговых стратегий

Интересное мнение про ООП

fxsaber, 2021.01.29 13:39

Портянка.

#property strict
#property script_show_inputs

input double inPerformance = 100; // Сколько процентов дает система за период
input double inGift = 50;         // Награда управляющего в процентах (< 100)
input int inN = 12;               // Сколько расчетных периодов

struct BASE
{
  double Investor;
  double Manager;
  double Summary;

  // Если управляющий снимает деньги в конце каждого расчетного периода.
  void Set1( const double Performance, const double Gift, const int N )
  {
    this.Investor = ::MathPow(1 + (Performance - 1)* (1 - Gift), N);
    this.Manager = (this.Investor - 1) * Gift / (1 - Gift);
    this.Summary = this.Investor + this.Manager;

    return;
  }

  // Если ничего не делалось в расчетные периоды.
  void Set2( const double Performance, const double Gift, const int N )
  {
    this.Summary = ::MathPow(Performance, N);
    this.Manager = (this.Summary - 1) * Gift;
    this.Investor = this.Summary - this.Manager;

    return;
  }

  // Если управляющий снимает деньги в конце каждого расчетного периода и реинвестирует их.
  void Set3( const double Performance, const double Gift, const int N )
  {
    this.Summary = ::MathPow(Performance, N);
    this.Investor = ::MathPow(1 + (Performance - 1)* (1 - Gift), N);
    this.Manager = this.Summary - this.Investor;

    return;
  }

  void operator *=( const double Deposit = 1 )
  {
    this.Investor *= Deposit;
    this.Manager *= Deposit;
    this.Summary *= Deposit;

    return;
  }
#define TOSTRING(A) #A + " = " + ::DoubleToString(A, 4) + " "
  string ToString( const bool FlagName = true ) const
  {
    return(FlagName ? TOSTRING(Investor) + TOSTRING(Manager) + TOSTRING(Summary)
                    : ::StringFormat("||%-12.4f||%-12.4f||%-12.4f||", this.Investor, this.Manager, this.Summary));
  }
#undef TOSTRING

};

struct PAMM
{
  double Performance; // Доходность за расчетный период
  double Gift;        // Вознагражение управляющего.
  int N;              // Сколько расчетных периодов.

  BASE Base1; // Если управляющий снимает деньги в конце каждого расчетного периода.
  BASE Base2; // Если ничего не делалось в расчетные периоды.
  BASE Base3; // Если управляющий снимает деньги в конце каждого расчетного периода и реинвестирует их.

  void Set( const double dPerformance, const double dGift, const int iN )
  {
    this.Performance = dPerformance;
    this.Gift = dGift;
    this.N = iN;

    this.Base1.Set1(1 + this.Performance / 100, this.Gift / 100, this.N);
    this.Base2.Set2(1 + this.Performance / 100, this.Gift / 100, this.N);
    this.Base3.Set3(1 + this.Performance / 100, this.Gift / 100, this.N);
  }

  void operator *=( const double Deposit = 1 )
  {
    this.Base1 *= Deposit;
    this.Base2 *= Deposit;
    this.Base3 *= Deposit;

    return;
  }

  string GetDescription( void ) const
  {
    return("Доходность за расчетный период " + ::DoubleToString(this.Performance, 1) + "%\n" +
           "Вознагражение управляющего " + ::DoubleToString(this.Gift, 1) + "%\n" +
           "Количество расчетных периодов. " + (string)this.N);
  }

  string ToString( const bool FlagName = true, const bool FlagDescription = true ) const
  {
    return(FlagDescription ? "Данные для инвестиционного счета со следующими характеристиками:\n\n" + this.GetDescription() +
                             "\n\nКонечный результат для Инвестора, Управляющего и Суммарно:"
                             "\n\nЕсли управляющий снимает деньги в конце каждого расчетного периода (1).\n" + this.Base1.ToString(FlagName) +
                             "\n\nЕсли ничего не делалось в расчетные периоды (2).\n" + this.Base2.ToString(FlagName) +
                             "\n\nЕсли управляющий снимает деньги в конце каждого расчетного периода и реинвестирует их (3).\n" + this.Base3.ToString(FlagName)
                           : this.Base1.ToString(FlagName) + this.Base2.ToString(FlagName) + this.Base3.ToString(FlagName));
  }
};

void OnStart()
{
  PAMM Pamm;

  Pamm.Set(inPerformance, inGift, inN);

  Print(Pamm.ToString());

  string Str = Pamm.GetDescription() + "\n   ";

  for (int i = 1; i <= 3; i++ )
    Str += ::StringFormat("||%-12s||%-12s||%-12s||", "Investor" + (string)i, "Manager" + (string)i, "Summary" + (string)i);

  for (int i = 1; i <= inN; i++ )
  {
    Pamm.Set(inPerformance, inGift, i);

    Str += StringFormat("\n%-2d:", i) + Pamm.ToString(false, false);
  }

  Print(Str);
}

Формально, наверное, это ООП. Но самая его примитивная часть. Однако, без нее не смог. Возможно, тупо мозг перестроился на другое и клепаю такое.

Советники: Price Impulse 2 Новая версия платформы MetaTrader Особенности языка mql5, тонкости

Интересное мнение про ООП - страница 11