Вопрос обзора кода - разрешить ли мне передачу auto_ptr в качестве параметра? - PullRequest
5 голосов
/ 09 августа 2010

Рассмотрим следующий пример кода, который я недавно видел в нашей кодовой базе:

void ClassA::ExportAnimation(auto_ptr<CAnimation> animation)
{
... does something
}

// calling method:
void classB::someMethod()
{
  auto_ptr<CAnimation> animation (new CAnimation(1,2));
  ClassA classAInstance;
  classAInstance.ExportAnimation(animation)
  ... do some more stuff
}

Мне это не нравится - и я бы лучше написал это так:

void ClassA::ExportAnimation(CAnimation* animation)
{
    ... does something
}

// calling method:
void classB::someMethod()
{
  auto_ptr<CAnimation> animation (new CAnimation(1,2));
  ClassA classAInstance;
  classAInstance.ExportAnimation(animation.get())
  ... do some more stuff
}

а это действительно проблема?

Ответы [ 4 ]

4 голосов
/ 09 августа 2010

Все зависит от того, что такое ExportAnimation и как оно реализовано.

Использует ли он только объект во время вызова, а затем оставляет его?

Затем преобразует в ссылку и передает реальную ссылку.Нет необходимости передавать членство, и аргумент не является обязательным, поэтому достаточно void ExportAnimation( CAnimation const & ).Преимущество состоит в том, что из интерфейса ясно, что у метода нет проблем с управлением памятью, он просто использует переданный объект и оставляет его как таковой.В этом случае передача необработанного указателя (как в предложенном вами коде) намного хуже, чем передача ссылки, поскольку неясно, отвечает ли ExportAnimation за удаление переданного объекта.

Сохраняет ли он объект для последующего использования?

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

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

Вы добавили некоторые проблемы в качестве комментария к другому ответу:

Могу ли я увидеть, что объект больше не существует после вызова метода?

Вызывающий auto_ptr сбрасывается в 0 в вызове, поэтому любая разыменование будет уничтоженоошибка и будет помечена в первом тесте, который вы попробуете.

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

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

Я ожидаю, что объект будет существовать, пока auto_ptr не выйдет из области видимости.

Стандарт auto_ptr, в отличие от boost::scope_ptr, не имеет этой семантики,Право собственности на объект может быть передано или передано другому auto_ptr, поэтому предположение, что объект, содержащийся в auto_ptr, живет для всей области действия auto_ptr, само по себе является плохим.

4 голосов
/ 09 августа 2010

auto_ptr однозначно заявляет, что владение указателем передано.Простой указатель не самодокументируется.

3 голосов
/ 09 августа 2010

Какой смысл авто-ptr, если вы используете его внутреннее устройство только как место хранения?

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

Похоже, что альтернатива, которую вы ищете, гораздо проще:

void ClassA::ExportAnimation(CAnimation &animation) // no pointer

// calling method:
void classB::someMethod()
{
  CAnimation animation(1,2); // no pointer
  ClassA classAInstance;
  classAInstance.ExportAnimation(animation) // no ownership tranfer
  ... do some more stuff
  // object dies here, no earlier, no later
}
1 голос
/ 09 августа 2010

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

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

Я бы всегда рекомендовал использовать умные указатели (и другие идиомы RAII), чтобы сделать время жизни объекта явным и автоматическим.

...