#define SAFE_DELETE(x) if(x != nullptr){ delete x; x = nullptr;} という行為は正解なのかどうか？

Question

提示コードの#define 部ですが二重deleteは良くないので#defineを使って防いでいるのですがこれは正解なのでしょうか？関数で書くと型の問題で汎用性が損なるのでこうしました。 提示コードの下のEntryクラスですが使い方としてif()文を使ってもしnullptrじゃないとき等を使って分岐するとコードが見ずらくなるので使ってみましたが正しいのでしょうか？

#include <iostream>
#include <cassert>


//#define NDEBUG

#define SAFE_DELETE(x) if(x != nullptr){ delete x; x = nullptr;}
int main()
{
	int* a = new int();
	
	SAFE_DELETE(a);
	SAFE_DELETE(a);

	return 0;
}

#include "Entry.hpp"
#include "Title.hpp"
#include "Game.hpp"
#include "GameOver.hpp"




Entry::Entry()
{
	changeScene = Scene::Title;

	title = nullptr;		//タイトル
	game = nullptr;			//ゲーム
	gameOver = nullptr;		//ゲームオーバー
}

void Entry::Loop()
{

	//シーン推移
	if (title != nullptr)
	{
		title->Loop(this);
	}
	else if (game != nullptr)
	{
		game->Loop(this);
	}
	else if (gameOver != nullptr)
	{
		gameOver->Loop(this);
	}



	//シーン削除
	switch (changeScene)
	{
		case Scene::Title:
		{
			SAFE_DELETE(game);
			SAFE_DELETE(gameOver);
			title = new Title();
		};
		break;

		case Scene::Game:
		{
			SAFE_DELETE(title);
			game = new Game();
		};
		break;

		case Scene::GameOver:
		{
			SAFE_DELETE(game);
			gameOver = new GameOver();
		};
		break;
	}
}

	//シーン切り替え
void Entry::MoveTo(Scene e)
{
	changeScene = e;
}

Comments

[退会済みユーザー]

現在質問文を編集中です。

Answer 1

二重deleteは良くないので#defineを使って防いでいるのですがこれは正解なのでしょうか？

ご自身の意図した通りに動くのであれば、その意味で「不正解」ではないでしょう。

しかし、一般的にその書き方は「正解」とは言えないと思います。
理由は2つ。

1つ目。「関数で書くと型の問題で汎用性が損なるので」と理由を挙げられていますが、テンプレートを使えば次の例のように十分に関数で書け、マクロを避けることができます。なので、#define で敢えて書く意義がありません。

template <typename T>
static inline void SAFE_DELETE(T* &p) {
  if ( p != nullptr ) {
    delete p;
    p = nullptr;
  }
}

2つ目。今のC++(C++11～)には std::uniq_ptr 等のスマートポインタと呼ばれるライブラリが標準であります。「安全性」を重視したいのなら、素のポインタで new,delete して自前で何か策を講じるのではなく、標準で用意されているそういった機能を使うべきです。

Answer 2

nullptrじゃないとき等を使って分岐するとコードが見ずらくなるので使ってみましたが正しいのでしょうか？

ほとんど意味がありません。delete nullptr;は何もしないことが保証されています（参考）。

Comments

[maisumakun]

同様に、free(NULL)も安全に何もしないことが保証されています。