C ++ семантика константных ссылок? - PullRequest
6 голосов
/ 11 июня 2010

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

#include <iostream>

using namespace std;

struct B
{
 B() : m_value(1) {}

 long m_value;
};

struct A
{
 const B& GetB() const { return m_B; }

 void Foo(const B &b)
 {
  // assert(this != &b);
  m_B.m_value += b.m_value;
  m_B.m_value += b.m_value;
 }

protected:
 B m_B;
};

int main(int argc, char* argv[])
{
 A a;

 cout << "Original value: " << a.GetB().m_value << endl;

 cout << "Expected value: 3" << endl;
 a.Foo(a.GetB());

 cout << "Actual value: " << a.GetB().m_value << endl;

 return 0;
}

Вывод:
Исходное значение: 1
Ожидаемое значение: 3
Фактическое значение: 4

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

Мой вопрос: Каким константным правилам следует следовать при разработке геттеров / сеттеров?

Мое предложение: никогда не возвращайте ссылку на переменную-член, если она может быть установлена ​​посредством ссылки через функцию-член.Следовательно, либо вернуть по значению , либо передать параметры по значению.(Современные компиляторы все равно оптимизируют лишнюю копию.)

Ответы [ 5 ]

20 голосов
/ 11 июня 2010

Очевидно, что программист одурачен константой b

Как кто-то однажды сказал, Вы продолжаете использовать это слово.Я не думаю, что это означает, что вы думаете, что это означает.

Const означает, что вы не можете изменить значение.Это не значит, что значение не может измениться.

Если программиста одурачивает тот факт, что какой-то другой код может изменить что-то, что они не могут, им нужно лучшее понимание псевдонимов.

Если программиста одурачивает тот факт, чтотокен «const» звучит немного как «константа», но означает «только для чтения», им нужно лучше понять семантику языка программирования, который они используют.

Так что если у вас есть метод получения, который возвращает constссылка, тогда это псевдоним для объекта, который вы не имеете права изменять.Это говорит ничего о том, является ли его значение неизменным.


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

Если вы сделаете B.m_value закрытым, то вы не сможете написать Fooу тебя есть.Вы можете либо сделать Foo в:

void Foo(const B &b)
{
    m_B.increment_by(b);
    m_B.increment_by(b);
}

void B::increment_by (const B& b)
{
    // assert ( this != &b ) if you like 
    m_value += b.m_value;
}

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

void Foo(B b)
{
    m_B.increment_by(b);
    m_B.increment_by(b);
}

Теперь приращение значения само по себе можетили может быть нецелесообразным, и его легко проверить в B :: increment_by.Вы также можете проверить, есть ли &m_b==&b в A :: Foo, хотя, если у вас есть пара уровней объектов и объектов со ссылками на другие объекты, а не на значения (поэтому &a1.b.c == &a2.b.c не означает, что &a1.b == &a2.b или &a1 == &a2), тогда вы действительно должны просто знать, что любая операция потенциально является псевдонимом.

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

Передача аргументов, которые имеют наименьшую структуру, также работает хорошо.Если Foo () использует long, а не объект, от которого он должен получить long, то он не будет иметь псевдонимов, и вам не нужно будет писать другой Foo () для увеличения m_b на значение C.

2 голосов
/ 11 июня 2010

Я предлагаю немного другое решение, которое имеет несколько преимуществ (особенно в многопоточном мире, который постоянно расширяется).Это простая идея, которой нужно следовать, а именно: «зафиксировать» ваши изменения последними.

Чтобы объяснить на своем примере, вы просто изменили бы класс 'A' на:

struct A
{
 const B& GetB() const { return m_B; }

 void Foo(const B &b)
 {
  // copy out what we are going to change;
  int itm_value = m_b.m_value;

  // perform operations on the copy, not our internal value
  itm_value += b.m_value;
  itm_value += b.m_value;

  // copy over final results
  m_B.m_value = itm_value ;
 }

protected:
 B m_B;
};

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

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

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

Существует много разных вещей, которые могут произойти с опубликованным исходным кодом.Я не удивлюсь, если некоторые компиляторы (с включенной оптимизацией) действительно будут генерировать код, который даст «ожидаемый» результат, тогда как другие не будут.Все это просто потому, что точка, в которой переменные, которые не являются «изменчивыми», фактически записываются / читаются из памяти, не очень хорошо определена в стандартах c ++.

1 голос
/ 11 июня 2010

Честно говоря, A::Foo() теряет меня не так, как ваша первоначальная проблема. В любом случае, я смотрю на это, это должно быть B::Foo(). А внутри B::Foo() проверка на this не будет такой диковинной.

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

Из прошлого опыта я бы отнесся к этому как к простой ошибке и выделил бы два случая: (1) B маленький и (2) B большой. Если B мало, просто сделайте A::getB(), чтобы вернуть копию. Если B велико, у вас нет другого выбора, кроме как обрабатывать случай, когда объекты B могут быть как rvalue, так и lvalue в одном и том же выражении.

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

1 голос
/ 11 июня 2010

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

Если, например, Foo было указано со значением-аргументом i.s.o. ссылочный аргумент, никакой проблемы не показал бы.

0 голосов
/ 11 июня 2010

Мой глупый ответ, я оставляю его здесь на всякий случай, если кто-то еще придумает такую ​​же плохую идею:

Проблема в том, что я думаю, что упомянутый объект не является const (B const & против const B &), только ссылка является константой в вашем коде.

...