Натягиваем ФП на ООП
Некоторое время назад, вернувшись после полугодового отпуска в функциональном мире, назад в ООП, я в который раз наступил на привычные грабли: случайно изменил состояние.
private double fBm(Vector2D v, int y)
{
double result = 0f;
double freq = Frequency;
for (int i = 0; i < Octaves; ++i)
{
result += NoiseFn(permutation, v * freq) * Amplitude;
freq *= Lacunarity;
Amplitude *= Gain; // <-- Вот тут.
}
return result;
}В ФП нужно особо постараться чтобы получить такой баг, а в некоторый языках невозможно в принципе. Салат из полезной работы и состояния класса не радовал, простор для ошибок даже в этой четверке строк слишком широк. Я стал думать как можно уменьшить площадь этих грабель и вывел следующее:
Во-первых нужно сделать this обязательным. Поставить поле не с той стороны выражения слишком легко. Если бы я везде явно указал контекст, скорее всего сразу бы заметил, что ступил. К сожалению, компилятор C# нельзя заставить требовать this, а в статических проверках студии есть только правило которое работает наоборот, то есть заставляет убирать this. Так что, видимо придется писать свое.
Во-вторых нужно исключить использование полей класса из тела метода, а все изменения записывать в конце метода, если они есть. У нас получается три абзаца. В первом мы объявляем все что нам понадобится, во втором делаем полезную работу, в третьем сохраняем состояние.
public void DoAThing(int input)
{
// Первый абзац
int a = this.A;
int b = this.B;
int result = 0;
// Второй абзац
for (int i = 0; i < input; ++i)
result += (a + b) * i;
// Третий абзац
this.Thing = result;
}В первом абзаце все состояние которое нужно мы сохраняем локально.
Во втором, this не фигурирует.
В третьем this всегда должен быть первым словом. Тут же будут вызываться другие методы которые меняют состояние.
return можно добавить в любое место, на поведение метода он не повлияет.
Плюсы
- Багов со случайным изменением состояния станет меньше.
- Отсутствие третьего абзаца делает метод чистой функцией, а значит его можно запускать параллельно.
Минусы
- Дисциплина. Фу-фу-фу. Без автоматических проверок, легко все испортить.
- В некоторых случаях этот подход будет работать медленней.
- Может выглядеть странно или глупо. Вот к примеру:
{ int a = this.A; int b = this.B; return a + b; }
Вместо заключения
Как думаете? В этом есть смысл или я парюсь?
Если вы уже делаете подобное или натягиваете другие части ФП на ООП, пожалуйста поделитесь.
Комментарии (18)
2 января 2017 в 16:38
0↑
↓
Есть языки, в которых this обязателен.
Что касается случайного изменения — думаю, достаточно добавить в юнит-тесты проверку на неизменность внутреннего состояния после вызова метода.
2 января 2017 в 16:48
0↑
↓
Юнит-тесты проверят правильность выполнения, а не неправильность. Метод может переписывать состояние только на части домена параметров. И для того что-бы написать тест, который вызовет это поведение, вам нужно будет изучить код метода.2 января 2017 в 19:17
0↑
↓
О, когда часть состояния мутабельная, а часть — нет, это создает огромную путаницу само по себе, особенно при отсутствии языковой поддержки иммутабельности.
Лучше выделить immutable value objects.
2 января 2017 в 19:40
0↑
↓
Я имел ввиду, что метод может изменять состояние только при определенных входных параметрах. Скажем если метод принимает два инта и если они оба простые числа, то тогда что-то ломается. Такие вещи тяжело проверить юнитами. Даже контракты, которые предназначены именно для поиска кривого кода, могут не словить это до продакшена. Я хочу предотвратить проблему еще до компиляции.
А то что вы описали — правда. Но иногда неизменность должна поддерживаться во временных рамках. Скажем как в моем коде, где я ошибся. Нет проблемы если амплитуда поменяется до или после, главное не во время. Было бы классно, если бы можно было помечать методы, которые не имеют права менять состояние. А еще лучше помечать те, которые имеют.
maaGames
2 января 2017 в 17:08
0↑
↓
Поздравляю, вы изобрели «транзакции»!Вообще, это довольно хорошая практика программирования, исключающая порчу состояния объекта при экстренном завершении метода. Но он снижает перформанс, так что надо выбирать.
Сразу дам совет. Гораздо проще и удобнее создавать не отдельные поля, а целую копию объекта в «абзац 1» и присваивать копию оригиналу в «абзац 3». Тогда вам безразлично наличие/отсутствие this, т.к. в левой части у вас должен быть модифицируемый объект (который копия). Думаю, говорить о бессбойном конструкторе копирования и присваивания даже упоминать не обязательно.
2 января 2017 в 17:56
0↑
↓
Как можно присвоить объект оригиналу?
this = ...сработает только в структуре, если мы говорим про шарп.-
maaGames
2 января 2017 в 18:19
0↑
↓
В C# так нельзя? Ну, тогда метод Copy наверняка сделать можно.
В С++ без проблем можно *this = a; написать. Удобно, пока не отстрелишь что-нибудь…2 января 2017 в 18:43
0↑
↓
Нельзя. Можно сделать так:
public void MakeThing(Something ref obj) { var thing = new Something(obj); // ... obj = thing; }Но в шарпе фокусы с ссылками и особенно оказателями используются только в крайнем случае. Для указателей даже компилятору нужно сказать, что если что — сам дурак.
2 января 2017 в 23:03
0↑
↓
В C++ есть идиома с вызовом Swap в конце метода, хорошо сочетается с идиомой pImpl (данные объекта хранятся в отдельном блоке — «реализации», а сам объект — «фасад» — обёртка над смартпойнером на этот блок). Дополнительные бонусы — простая дисциплина потоковой безопасности (надо позаботиться только о безопасности Swap) и сильная безопасность по исключениями (объект никогда не остаётся в некорректном «смешанном» состоянии, если посреди метода происходит исключение, при этом требование noexcept необходимо только для Swap). Эта идиома полуофициально поддерживается в STL (см. реализацию stl: swap для классов библиотеки плюс для POD типов).При использовании этой идиомы дисциплину можно организовать такую. Объекты-реализации никогда не имеют методов, меняющих состояние, но, зато, имеют достаточно богатые наборы конструкторов. Основное тело метода объекта public API (фасада соответствующего объекта-реализации) состоит в построении новой версии объекта-реализации (при этом копировать поля текущего объекта в локальные переменные, как у автора поста, смысла нет — компилятор в помощь, поскольку pImpl-смартпойнтер фасада указывает на константный объект, да и все методы объекта-реализации помечены const). Эта часть метода завершается вызовом конструктора нового объекта-реализации, с сохранением указателя на него в локальную переменную — такой же смартпойнтер, как pImpl. Заключительный оператор метода — swap (std: swap, для единообразия) этой переменной и единственного поля фасада — pImpl.
Кстати, роль явного this (как у автора поста) здесь играет необходимость обращаться к реализации через единственное поле фасада — pImpl.
Дополнительная стоимость такого подхода, применяемого строго, понятна —, но в каких-то применениях она оправдана, а узкие по производительности места можно и подрихтовать, если надо.
2 января 2017 в 17:40
+1↑
↓
нельзя заставить требовать this
Так что, видимо придется писать свое
В stylecop уже есть соответствующее правило
GennPen
2 января 2017 в 18:56
0↑
↓
Можно использовать в классе что-то типа такого:public int VariableWrite { get; set; } public int VariableReadOnly => VariableWrite;
… чтобы ясно было видно с каким доступом обращаешься к переменной.И странно что у вас в примере Frequency скопирована, а Amplitude нет.
2 января 2017 в 19:23
0↑
↓
И странно что у вас в примере Frequency скопирована, а Amplitude нет.
В этом то и есть проблема. Просто необратил внимание.
Ваш вариант будет работать быстрее. Я только бы вариант для чтения назвал без суффикса, что-бы он привлекательней выглядел. Смущает только наличие
this.
2 января 2017 в 19:03
0↑
↓
Ну еще обычно к имени аттрибута класса добавляют к примеру «m» спереди. Многим кажется лишним, но мне часто помогает определиться, особенно если наворотят метод на пару страниц.2 января 2017 в 19:26
0↑
↓
Я раньше тоже не любил приставок, но думаю что сейчас начну добавлять подчеркивание для локальных переменных, что бы не портить внешний вид класса.
-
GennPen
2 января 2017 в 19:45
0↑
↓
У вас в примере итак уже нормально именуются переменные: локальные — с маленькой буквы, класса — с большой, а с подчеркивания обычно private именуются.2 января 2017 в 20:00
0↑
↓
Я просто думаю что все свойства класса я могу увидеть в автодополнении если напишу
this., а все локальные через подчеркивание. И подчеркивание лучше видно, чем m. И с областью видимости согласуется: У пабликов — большая буква, у приватов — маленькая, у локальных — никакая.
qw1
2 января 2017 в 19:39 (комментарий был изменён)
0↑
↓
Чтобы реализовать проверку правила №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…
impwx
2 января 2017 в 19:59
+2↑
↓
То, что вы пытались изобрести, называется «чистые методы». В C# есть способ реализации куда лучше: делайте как можно больше методов статическими. Тогда в них гарантию неизменяемости объекта вам даст сам компилятор, и тестировать эти методы станет проще. В качестве точек композиции, в которых состояние меняется, останутся немногочисленные экземплярные методы и конструктор.
