Pull to refresh

Comments 34

Есть языки, в которых this обязателен.


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

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

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


Лучше выделить immutable value objects.

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


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

Поздравляю, вы изобрели «транзакции»!

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

Сразу дам совет. Гораздо проще и удобнее создавать не отдельные поля, а целую копию объекта в «абзац 1» и присваивать копию оригиналу в «абзац 3». Тогда вам безразлично наличие/отсутствие this, т.к. в левой части у вас должен быть модифицируемый объект (который копия). Думаю, говорить о бессбойном конструкторе копирования и присваивания даже упоминать не обязательно.

Как можно присвоить объект оригиналу? this = ... сработает только в структуре, если мы говорим про шарп.

В C# так нельзя? Ну, тогда метод Copy наверняка сделать можно.
В С++ без проблем можно *this = a; написать. Удобно, пока не отстрелишь что-нибудь…

Нельзя. Можно сделать так:


public void MakeThing(Something ref obj)
{
    var thing = new Something(obj);
    // ...
    obj = thing;
}

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

В C++ есть идиома с вызовом Swap в конце метода, хорошо сочетается с идиомой pImpl (данные объекта хранятся в отдельном блоке — «реализации», а сам объект — «фасад» — обёртка над смартпойнером на этот блок). Дополнительные бонусы — простая дисциплина потоковой безопасности (надо позаботиться только о безопасности Swap) и сильная безопасность по исключениями (объект никогда не остаётся в некорректном «смешанном» состоянии, если посреди метода происходит исключение, при этом требование noexcept необходимо только для Swap). Эта идиома полуофициально поддерживается в STL (см. реализацию stl::swap для классов библиотеки плюс для POD типов).

При использовании этой идиомы дисциплину можно организовать такую. Объекты-реализации никогда не имеют методов, меняющих состояние, но, зато, имеют достаточно богатые наборы конструкторов. Основное тело метода объекта public API (фасада соответствующего объекта-реализации) состоит в построении новой версии объекта-реализации (при этом копировать поля текущего объекта в локальные переменные, как у автора поста, смысла нет — компилятор в помощь, поскольку pImpl-смартпойнтер фасада указывает на константный объект, да и все методы объекта-реализации помечены const). Эта часть метода завершается вызовом конструктора нового объекта-реализации, с сохранением указателя на него в локальную переменную — такой же смартпойнтер, как pImpl. Заключительный оператор метода — swap (std::swap, для единообразия) этой переменной и единственного поля фасада — pImpl.

Кстати, роль явного this (как у автора поста) здесь играет необходимость обращаться к реализации через единственное поле фасада — pImpl.

Дополнительная стоимость такого подхода, применяемого строго, понятна — но в каких-то применениях она оправдана, а узкие по производительности места можно и подрихтовать, если надо.
Я бы не стал предлагать этот подход как универсальное решение потокобезопасности. Segfault из-за конкурентного доступа мы так не получим, но вот логика приложения может пострадать. Всё-таки, не зря классический подход — это Lock на всё время работы функции, выполняющей изменение данных.
Согласен, но это один из разумных компромиссов для тех случаев, когда Lock в каждом модифицирующем методе — дорог по производительности. Бескомпромисный вариант — делать все структуры данных, разделяемые между потоками, immutable, с единственным корнем, который обновляется atomic операцией. Но это тоже дорого иногда (из-за дороговизны immutable версий сложных структур данных).
Это тоже не выход. Вот дерево:
[корень]
|→[Нода1] Значение:50
|→[Нода2] Значение:30

Если два потока параллельно хотят увеличить значение в ноде1 на 10 и 20 соответственно, то переприсвоение корня оставит результат только одной операции, а вторая потеряется.

Есть, конечно, атомарный compare-and-swap (CAS). Это когда мы заранее запоминает указатель на корень, а при замене сравниваем с прошлым значением и, если корень изменился, swap не делаем, а откатываем всё операцию и начинаем заново.

Но CAS на весь корень это очень дорого (т.к. по сути успешно завершится модификация только из одного потока, а остальные будут впустую жечь электричество). Тут честный Lock на всё время операции экономнее.
Но CAS на весь корень это очень дорого (т.к. по сути успешно завершится модификация только из одного потока, а остальные будут впустую жечь электричество). Тут честный Lock на всё время операции экономнее.
В этом и суть CAS операций и не важно корень это или операции мутации по отдельности. В качестве бонуса мы получаем lock-free реализацию, которую получить на lock-ах по определению нельзя.
CAS на корень — это гарантированный худший сценарий, потому как любое параллельное действие сделает откат всего, что сделано в последней транзакции, без шансов немного приостановиться и подождать.

Цель какая — эффективность или любым способом заюзать модный lock-free?

Оптимистичные алгоритмы (CAS) хороши, когда конфликт редко случается. Пессимистичные (lock) — когда часто. У каждого своя область применения.

И главное, на CAS не будет честной очереди. Длинная операция всегда будет откатываться, если не прекращаются короткие запросы. Lock ставит всех в одну очередь.
В этом и суть CAS операций и не важно корень это или операции мутации по отдельности.
Как это не важно? Вот пример дерева в посте выше.
Допустим, это список финансовых счетов. Один поток переносит 10 рублей из ноды1 в ноду2.
В ноде1 CAS успешно завершился, а в ноде2 показал, что было изменение. Всё, приехали — данные неконсистентны.
Никогда не рассматривал этот метод в контексте потоковой безопасности. Для меня его основное преимущество в сохранении инварианта, если «что-то пошло не так» и нужно сохранить исходное состояние объекта. Для многопоточности lock как-то безопаснее в общем случае, согласен.
нельзя заставить требовать this
Так что, видимо придется писать свое
В stylecop уже есть соответствующее правило
Можно использовать в классе что-то типа такого:
        public int VariableWrite { get; set; }
        public int VariableReadOnly => VariableWrite;

… чтобы ясно было видно с каким доступом обращаешься к переменной.

И странно что у вас в примере Frequency скопирована, а Amplitude нет.
И странно что у вас в примере Frequency скопирована, а Amplitude нет.

В этом то и есть проблема. Просто необратил внимание.


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

Ну еще обычно к имени аттрибута класса добавляют к примеру «m» спереди. Многим кажется лишним, но мне часто помогает определиться, особенно если наворотят метод на пару страниц.

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

У вас в примере итак уже нормально именуются переменные: локальные — с маленькой буквы, класса — с большой, а с подчеркивания обычно private именуются.

Я просто думаю что все свойства класса я могу увидеть в автодополнении если напишу this., а все локальные через подчеркивание. И подчеркивание лучше видно, чем m. И с областью видимости согласуется: У пабликов — большая буква, у приватов — маленькая, у локальных — никакая.

Вот как раз хотел написать — к чему усложнения с обязательным this, если можно обойтись правилами именования аттрибутов: наглядно, легко контролировать и очень непросто спутать с той же локальной переменной.
Чтобы реализовать проверку правила №2 средствами компилятора, функции могут быть статическими. Невозможно будет получить доступ к полям класса. Скоро c# 7 в этом поможет, позволив писать довольно чисто:
public class SomeClass
{
    int A;
    int B;

    private static (int,int) ComputeState(int a, int b) { return (a+b, a-b); }

    public void ChangeState() { (this.A, this.B) = ComputeState(this.A, this.B); }
}


Ещё обещают локальные ф-ции, если бы они тоже понимали static…
То, что вы пытались изобрести, называется «чистые методы». В C# есть способ реализации куда лучше: делайте как можно больше методов статическими. Тогда в них гарантию неизменяемости объекта вам даст сам компилятор, и тестировать эти методы станет проще. В качестве точек композиции, в которых состояние меняется, останутся немногочисленные экземплярные методы и конструктор.
Вариант: каждый тип — struct (чтобы успокоить себя насчёт производительности) со всеми полями readonly и удобным набором конструкторов. Все методы — чистые (можно для единообразия делать их все extension-методами, пополняя набор методов без вмешательства в определение struct-«ядра» библиотеки; дополнительный плюс такого правила — обязательное именование this параметра в реализации каждого такого extension-метода, как хочет автор этого поста); те из них, которые «мутаторы», возвращают новую («преобразованную») копию struct this-параметра (первого параметра в реализации extension-метода). Бонус: при такой дисциплине цепочечный синтаксис обеспечен для каждого типа «из коробки»: myS = myS.Transform1().Transform2().Transform3();. Лишнее копирование полей this-параметра в локальные переменные не требуется, дисциплина обеспечивает иммьютабельность автоматически. Правила для поддержания этой дисциплины при статической проверке вроде бы должны быть несложными.
А вы изобрели довольно изощренную реализацию монады. Но почему struct позволит «успокоиться насчет производительности»?
Потому что промежуточные данные будут в «горячей области» — не стеке, а не в непредсказуемой (по попаданию в кеш) куче.
Поставить поле не с той стороны выражения слишком легко.

Рандом драйвен девелопмент — расставляем переменные в случайном порядке, авось заработает?


У нас получается три абзаца. В первом мы объявляем все что нам понадобится, во втором делаем полезную работу, в третьем сохраняем состояние

Вы предлагаете безусловно получать все данные, которые могут потребоваться методу, даже если они фактически не потребуются из-за условия в середине "полезной работы"?


Отсутствие третьего абзаца делает метод чистой функцией,

Не делает, она остаётся недетерминированной.


а значит его можно запускать параллельно.

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

Рандом драйвен девелопмент

Это было бы смешно, если бы не было так грустно.


Вы предлагаете безусловно получать все данные, которые могут потребоваться методу, даже если они фактически не потребуются из-за условия в середине "полезной работы"?

Это и так происходит:


private int name(int a, int b, int c)
{
    if (b == 0)
        return a / c;
    return a / b;
}

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


Не делает, она остаётся недетерминированной.

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


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

В этом методе нет записи состояния.

Это и так происходит:

А так не происходит:


private int name()
{
    if( this.b == 0 )
        return this.a / this.c;
    return this.a / this.b;
}

И так не происходит:


private int name( lazy int a, lazy int b, lazy int c )
{
    if (b == 0)
        return a / c;
    return a / b;
}

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

Копирование в стек ничего не даст. Современные процессоры/компиляторы сами умеют предзагружать данные в кэш.


Если я что-то упустил, укажите.

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


В этом методе нет записи состояния.

Зато есть неатомарное чтение. Если вы запустите несколько таких функций в параллель, а тем временем начнёте менять состояние, то эти функции прочитают ерунду.

А так не происходит:

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


И так не происходит:

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


Копирование в стек ничего не даст. Современные процессоры/компиляторы сами умеют предзагружать данные в кэш.

Процессору нужно сильно помогать, что бы он подгружал нужные данные. Если ваша проблема чувствительна к data locality, то врядли вы будете использовать абстракции. Там все будет прибито гвоздями, отполировано и помечено, чтоб никто не трогал.


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

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


Зато есть неатомарное чтение. Если вы запустите несколько таких функций в параллель, а тем временем начнёте менять состояние, то эти функции прочитают ерунду.

Конечно. Все равно нужно лочить объект и тд. Но сам метод не напортачит, если запустить его параллельно.

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

Нет, там передаётся функция получения объекта, которая может быть заинлайнена компилятором.


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

У прибитых гвоздями алгоритмов очень плохо с переносимостью.


В рамках вопроса, можно опустить эту деталь.

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


Но сам метод не напортачит, если запустить его параллельно.

Он выдаст неправильный результат. Оно вам надо?

Sign up to leave a comment.

Articles