我有这个小片段:
Action* newAction(Agent& a, ACTION_MODE& m)
{
Action *new_action;
do
{
new_action = new Action(a,m);
}
while (!state->addAction(new_action));
return new_action;
}
state->addAction(Action *a);
如果添加了 *a 将返回 true,如果未添加 *a 则返回 false(检查 *a 是否已经存在)。
现在,我知道 goto 被很多人认为是邪恶的,所以,在上面的代码片段中,在每个循环中重新分配 new_action,而不删除它,这不是错误的吗?
下面的代码片段会不会更“明智”?
retry :
Action *new_action = new Action(a,m);
if (state->addAction(new_action))
{
return new_action;
}
else
{
delete new_action;
goto retry;
}
如果这看起来很初级,我很抱歉,但这是我一段时间以来一直想知道的事情。什么是删除内存然后重新分配,还是我可以立即重新分配?
编辑:
那会更好吗?
Action* newAction(Agent& a, ACTION_MODE& m)
{
// state will only allow an action to be added if it does not already exist
Action *new_action = new Action(a,m);
if (!new_action) return 0;
while (!state->addAction(new_action))
{
delete new_action;
new_action = new Action(a,m);
}
return new_action;
}
此函数的调用者需要一个已添加到状态的新操作,因此必须在此处进行删除操作。
最佳答案
您在该代码中有一个潜在的内存泄漏,因为您在每次循环中都创建了一个新 操作,但是如果添加失败,您不会释放它。更好的方法是将 Action 创建移动到循环之外,例如:
Action *newAction (Agent& a, ACTION_MODE& m) {
Action *new_action = new Action(a,m);
while (!state->addAction(new_action))
;
return new_action;
}
这比不断删除并重新创建操作更有效,应优先使用。
我还修复了返回类型,这样编译器就不会报错,如果您永远无法添加操作,您可能应该引入某种失败策略。
类似于:
Action *newAction (Agent& a, ACTION_MODE& m) {
// Try to make action, return null if no go.
Action *new_action = new Action(a,m);
if (new_action == 0)
return 0;
// Try a limited number of times to add action to state.
int limit = 10;
while (!state->addAction(new_action)) {
if (--limit == 0)
break;
// Optional sleep if desired.
}
// If that didn't work, delete action and return null.
if (limit == 0) {
delete new_action;
return 0;
}
// Otherwise the action was added, return it.
return new_action;
}
关于C++ 堆分配和内存重用,我们在Stack Overflow上找到一个类似的问题: https://stackoverflow.com/questions/6702426/