使用GOTO或不?
目前我正在开发一个项目,在那里使用goto语句。 goto语句的主要用途是在例程中有一个清理部分,而不是多个return语句。 如下所示:
BOOL foo() { BOOL bRetVal = FALSE; int *p = NULL; p = new int; if (p == NULL) { cout<<" OOM \n"; goto Exit; } // Lot of code... Exit: if(p) { delete p; p = NULL; } return bRetVal; }
这使得它更容易,因为我们可以在代码的一个部分跟踪我们的清理代码,也就是在Exit标签之后。
不过,我读过很多地方,有goto语句是不好的做法。
目前我正在阅读代码完整的书,它说,我们需要使用variables接近他们的声明。 如果我们使用goto,那么我们需要在第一次使用goto之前声明/初始化所有的variables,否则编译器会给出错误,因为goto语句跳过了xxvariables的初始化。
哪种方式是正确的?
从斯科特的评论:
它看起来像使用goto从一个部分跳到另一个是不好的,因为它使代码难以阅读和理解。
但是,如果我们使用goto只是向前和一个标签,那么它应该是罚款(?)。
我不确定清理代码是什么意思,但在C ++中有一个叫做“ 资源获取就是初始化 ”的概念,应该是你的析构函数清理东西的责任。
(请注意,在C#和Java中,这通常是通过try / finally来解决的)
欲了解更多信息,请查看此页面: http : //www.research.att.com/~bs/bs_faq2.html#finally
编辑 :让我澄清这一点点。
考虑下面的代码:
void MyMethod() { MyClass *myInstance = new MyClass("myParameter"); /* Your code here */ delete myInstance; }
问题 :如果你有多个退出函数会发生什么? 您必须跟踪每个出口,并在所有可能的出口处删除您的物品! 否则,你将有内存泄漏和僵尸资源,对不对?
解决scheme :改用对象引用,因为当控件离开作用域时会自动清理它们。
void MyMethod() { MyClass myInstance("myParameter"); /* Your code here */ /* You don't need delete - myInstance will be destructed and deleted * automatically on function exit */ }
哦,是的,并使用std::unique_ptr
或类似的东西,因为上面的例子显然是不完美的。
我从来没有在C ++中使用goto。 永远。 EVER。 如果有这样的情况应该使用,这是非常罕见的。 如果你实际上正在考虑把你的逻辑变成标准的一部分,有些东西已经从轨道上飞走了。
关于gotos和你的代码基本上有两点:
-
转到是坏的。 遇到一个你需要得到的地方是非常罕见的,但我不会build议完全打击它。 尽pipeC ++具有足够聪明的控制stream程,使得转到很less适合。
-
你的清理机制是错误的:这点更为重要。 在C中,使用自己的内存pipe理不仅是可以的,而且往往是最好的办法。 在C ++中,你的目标应该是尽可能地避免内存pipe理。 您应尽可能避免内存pipe理。 让编译器为你做。 而不是使用
new
,只是声明variables。 唯一一次你真的需要内存pipe理的是当你不知道你的数据的大小。 即使这样,你也应该尝试使用一些STL
集合。
如果你合法地需要内存pipe理(你还没有真正提供这方面的证据),那么你应该通过构造函数将内存pipe理封装在一个类中,以分配内存和解构器来释放内存。
你的回应是,你做事的方式要容易得多,从长远来看并不是真的。 首先,一旦你对C ++有了强烈的感觉,这样的构造函数将成为第二性质。 就我个人而言,我发现使用构造函数比使用清理代码更容易,因为我没有必要仔细注意确保我正确地释放。 相反,我可以让对象离开范围,语言为我处理。 而且,维护它们比维护一个清理部分容易得多,而且更不容易出现问题。
总之,在某些情况下, goto
可能是一个不错的select,但在这一个不是。 这只是短期的懒惰。
你的代码是非常非惯用的,你不应该写它。 你基本上用C ++模拟C语言。 但是其他人也提到过,并指出RAII是另一种select。
但是,您的代码将无法正常工作 ,因为:
p = new int; if(p==NULL) { … }
将永远不会评估为true
(除非你以怪异的方式重载了operator new
)。 如果operator new
不能分配足够的内存,它将抛出一个exception,它永远 不会返回0
,至less不会使用这组参数。 有一个特殊的位置 – 新的重载需要一个std::nothrow
types的实例,并且确实返回0
而不是引发exception。 但是这个版本很less用在普通的代码中。 一些低级代码或embedded式设备应用程序可以在处理exception情况太昂贵的环境中受益。
类似的情况也适用于你的delete
块,就像Harald所说: if (p)
在delete p
是不需要的。
另外,我不确定你的例子是否被故意select,因为这个代码可以被重写如下:
bool foo() // prefer native types to BOOL, if possible { bool ret = false; int i; // Lots of code. return ret; }
可能不是一个好主意 。
总的来说,从表面上看,你的方法没有什么不对,只要你只有一个标签,gotos总是前进。 例如,这个代码:
int foo() { int *pWhatEver = ...; if (something(pWhatEver)) { delete pWhatEver; return 1; } else { delete pWhatEver; return 5; } }
而这个代码:
int foo() { int ret; int *pWhatEver = ...; if (something(pWhatEver)) { ret = 1; goto exit; } else { ret = 1; goto exit; } exit: delete pWhatEver; return ret; }
实际上并不完全相同。 如果你能接受一个,你应该能够接受另一个。
然而,在很多情况下, RAII (资源获取是初始化)模式可以使代码更清洁,更易于维护。 例如,这个代码:
int foo() { Auto<int> pWhatEver = ...; if (something(pWhatEver)) { return 1; } else { return 5; } }
比以前的两个例子更短,更容易阅读,更易于维护。
所以,如果可以的话,我会推荐使用RAII方法。
你的例子不是exception安全的。
如果你正在使用goto来清理代码,那么如果在清理代码之前发生exception,那么它是完全错过的。 如果你声称你没有使用exception,那么你错了,因为new
内存不足时会抛出bad_alloc。
同样在这个时候(当bad_alloc被抛出时),你的堆栈将被解开,在调用堆栈的过程中,每个函数中都没有清除所有的清除代码,从而不能清理你的代码。
你需要看看对智能指针进行一些研究。 在上面的情况下,你可以使用std::auto_ptr<>
。
还要注意在C ++代码中,不需要检查一个指针是否为NULL(通常是因为你永远不会有RAW指针),但是因为new
不会返回NULL(它抛出)。
在C ++中也不像(C),在代码中看到早期的返回是很常见的。 这是因为RAII会自动进行清理,而在C代码中,您需要确保在函数的末尾添加特殊的清理代码(有点像代码)。
我想其他答案(和他们的意见)已经涵盖了所有重要的观点,但是这里有一件事情还没有做好:
你的代码应该是什么样子的:
bool foo() //lowercase bool is a built-in C++ type. Use it if you're writing C++. { try { std::unique_ptr<int> p(new int); // lots of code, and just return true or false directly when you're done } catch (std::bad_alloc){ // new throws an exception on OOM, it doesn't return NULL cout<<" OOM \n"; return false; } }
那么,它就更短了,就我所知,更正确的(正确处理OOM的情况),最重要的是,我不需要编写任何清理代码或做任何特别的事情来“确保我的返回值被初始化”。
当我写这个代码的时候,我只注意到了一个代码问题,那就是“现在bRetVal的价值是什么?”。 我不知道,因为这是上面宣布的waaaaay,这是最后一次分配给什么时候? 在这之上的某个点。 我必须通读整个函数,以确保我明白将要返回的内容。
我如何说服自己记忆被释放?
我怎么知道我们永远不会忘记跳到清理标签? 我必须从清理标签上倒退,find指向它的每一个转到,更重要的是find那些不在那里的转移。 我需要追踪函数的所有path,以确保函数得到正确清理。 这就像我的意大利面条码。
非常脆弱的代码,因为每次资源必须清理,你必须记得复制你的清理代码。 为什么不写一次,在需要清理的types? 然后依靠它被自动执行,每次我们需要它呢?
你应该从Linux内核邮件列表中读取这个线程摘要(特别注意Linus Torvalds的回复),然后再为goto
制定一个策略:
在我编程的八年里,我使用了很多,其中大部分是在我使用GW-BASIC版本的第一年,而且从1980年开始没有明确说明goto在某些情况下使用。 我唯一一次在C ++中使用goto的时候,我有如下的代码,我不知道是否有更好的方法。
for (int i=0; i<10; i++) { for (int j=0; j<10; j++) { if (somecondition==true) { goto finish; } //Some code } //Some code } finish:
我知道goto的唯一情况仍然是大型机汇编语言,我知道的程序员一定要logging代码跳转的地方以及为什么。
一般来说,你应该devise你的程序来限制对gotos的需求。 使用OO技术来“清理”你的返回值。 有办法做到这一点,不需要使用gotos或复杂的代码。 在某些情况下,gotos非常有用(例如,深度嵌套的范围),但是如果可能的话应该避免。
正如在Linux内核中所使用的,当单个函数必须执行2个或更多的步骤时,goto才能用于清理工作。 步骤不需要内存分配。 这可能是对一段代码或I / O芯片组寄存器的configuration更改。 只有在less数情况下才需要Goto,但是如果使用正确的话,它们可能是最好的解决scheme。 他们不是邪恶的。 他们是一个工具。
代替…
do_step1; if (failed) { undo_step1; return failure; } do_step2; if (failed) { undo_step2; undo_step1; return failure; } do_step3; if (failed) { undo_step3; undo_step2; undo_step1; return failure; } return success;
你可以用goto语句来做同样的事情:
do_step1; if (failed) goto unwind_step1; do_step2; if (failed) goto unwind_step2; do_step3; if (failed) goto unwind_step3; return success; unwind_step3: undo_step3; unwind_step2: undo_step2; unwind_step1: undo_step1; return failure;
应该清楚的是,鉴于这两个例子,一个比另一个更可取。 对于RAII人群来说……只要能保证展开总是按照相反的顺序发生,那么这种方法没有任何问题:3,2,1。最后,有些人在他们的代码中没有使用例外并指示编译器将其禁用。 因此,不是所有的代码都必须是exception安全
GOTO的缺点是相当讨论。 我只是补充一下:1)有时你必须使用它们,应该知道如何最小化问题; 2)一些可以接受的编程技巧是伪装的,所以要小心。
1)当你必须使用GOTO,比如在ASM或.bat文件中,像编译器一样思考。 如果你想编码
if (some_test){ ... the body ... }
做一个编译器。 生成一个标签,其目的是跳过身体,而不是做任何以下事情。 即
if (not some_test) GOTO label_at_end_of_body ... the body ... label_at_end_of_body:
不
if (not some_test) GOTO the_label_named_for_whatever_gets_done_next ... the body ... the_label_named_for_whatever_gets_done_next:
换句话说,标签的目的不是为了做某事,而是为了跳过某些东西。
2)我所谓的GOTO伪装是通过定义一对macros来变成GOTO + LABELS代码的东西。 一个例子是通过具有状态variables和while-switch语句来实现有限状态自动机的技术。
while (not_done){ switch(state){ case S1: ... do stuff 1 ... state = S2; break; case S2: ... do stuff 2 ... state = S1; break; ......... } }
可以变成:
while (not_done){ switch(state){ LABEL(S1): ... do stuff 1 ... GOTO(S2); LABEL(S2): ... do stuff 2 ... GOTO(S1); ......... } }
只需定义一对macros。 几乎任何FSA都可以变成结构化的无代码代码。 我宁愿远离伪装代码,因为它可以进入与未掩盖的gotos相同的意大利面条代码问题。
补充:只是为了让人放心:我认为一个优秀的程序员的一个标志正在认识到通用规则不适用的时候。
使用goto去清理部分会导致很多问题。
首先,清理部分容易出现问题。 它们具有较低的凝聚力(没有真正的angular色,可以用程序的内容来描述),高耦合性(正确性很大程度上依赖于其他代码段),并且完全不是exception安全的。 看看你是否可以使用析构函数进行清理。 例如,如果将int *p
更改为auto_ptr<int> p
,则指向的p将自动释放。
其次,正如你指出的那样,它会迫使你在使用之前很久就声明variables,这将使得更难理解代码。
第三,当你提出一个相当有纪律的使用goto的时候,会有一种宽松使用它的诱惑,然后代码将变得难以理解。
有很less的情况下goto是适当的。 大多数时候,当你想要使用它们时,这是一个信号,说明你做错了事情。
由于这是一个经典的话题,我将回答Dijkstra的被认为有害的Go-to声明 (最初发布在ACM中)。
Goto提供更好的不要重复自己 (DRY),当“尾端逻辑”对于一些非全部情况是常见的。 特别是在“switch”语句中,当一些switch-branches具有tail-end-commonality时,我经常使用goto。
switch(){ case a: ... goto L_abTail; case b: ... goto L_abTail; L_abTail: <commmon stuff> break://end of case b case c: ..... }//switch
您可能已经注意到,当您需要在例程中间进行尾端合并时,引入额外的大括号足以满足编译器的需求。 换句话说,你不需要在顶部声明一切; 这确实是较差的可读性。
... goto L_skipMiddle; { int declInMiddleVar = 0; .... } L_skipMiddle: ;
使用Visual Studio的更高版本检测未初始化variables的使用情况时,我发现自己总是初始化大多数variables,即使我认为它们可能在所有分支中都被分配 – 很容易编写一个“追踪”语句,该语句引用了一个从未分配过的variables因为你的思想不把追踪语句看作“真实的代码”,但是当然Visual Studio仍然会检测到一个错误。
除此之外,不要重复自己,将标签名称分配给这样的尾端逻辑,甚至似乎通过select好的标签名称来帮助我的头脑保持一致。 没有一个有意义的标签,你的评论可能会说同样的话。
当然,如果你实际上是在分配资源,那么如果auto-ptr不适合,那么你真的必须使用try-catch,但是在exception安全性的时候,tail-end-merge-don't-repeat-yourself会经常发生。没什么大不了。
总之,虽然goto可以用来编写类似意大利面条的结构,但是对于一些非全部情况而言,尾部序列是常见的,那么goto可以提高代码的可读性,甚至可维护性如果你以其他方式复制/粘贴,那么以后某个人可能会更新一个而非另一个。 所以,对教条的狂热可能会适得其反。
C中每个函数都有一个单一的出口点的习惯用法,就是把所有的清理工作放在一个地方。 如果使用C ++析构函数来处理清理操作,那就不再需要了 – 无论函数有多less个退出点,都将进行清理。 所以在合理devise的C ++代码中,不再需要这种types的东西。
从以前的所有评论:
- goto非常糟糕
- 它使代码难以阅读和理解。
- 它可以导致众所周知的问题“意大利面代码”
- 大家都同意不应该这样做。
但我在以下情况下使用
- 这是用来前进,只有一个标签。
- goto部分用于清理代码并设置返回值。 如果我不使用转到,那么我需要创build一个类的每个数据types。 就像我需要将int *包装到类中一样。
- 在整个项目中得到遵循。
我同意,这是不好的,但如果遵循正确的话,这样做会让事情变得更容易。
很多人用gotos吓坏了, 他们不是。 也就是说,你永远不会需要一个; 总是有一个更好的方法。
当我发现自己“需要”转到这种types的东西时,我几乎总是发现我的代码太复杂了,可以很容易地分解成几个方法调用,更容易阅读和处理。 您的调用代码可以执行如下操作:
// Setup if( methodA() && methodB() && methodC() ) // Cleanup
并不是说这是完美的,但是更容易遵循,因为所有的方法都将被命名,以清楚地表明问题可能是什么。
然而,通过阅读意见,应该表明你的团队比转到处理更紧迫的问题。
你给我们的代码是(几乎)C代码写在一个C ++文件。 你正在使用的内存清理types可以在没有使用C ++代码/库的C程序中使用。
在C ++中,你的代码是不安全和不可靠的。 在C ++中,您要求的pipe理方式有所不同。 使用构造函数/析构函数。 使用智能指针。 使用堆栈。 总之,使用RAII 。
你的代码可以(例如,在C ++中,应该)写成:
BOOL foo() { BOOL bRetVal = FALSE; std::auto_ptr<int> p = new int; // Lot of code... return bRetVal ; }
(请注意,在实际代码中新build一个int有点愚蠢,但是可以用任何types的对象replaceint,然后更有意义)。 假设我们有一个Ttypes的对象(T可以是一个int,一些C ++类,等等)。 然后代码变成:
BOOL foo() { BOOL bRetVal = FALSE; std::auto_ptr<T> p = new T; // Lot of code... return bRetVal ; }
甚至更好,使用堆栈:
BOOL foo() { BOOL bRetVal = FALSE; T p ; // Lot of code... return bRetVal; }
无论如何,上述任何示例都比您的示例更容易阅读和安全。
RAII有许多方面(即使用智能指针,堆栈,使用vector而不是可变长度数组等等),但总的来说,就是尽可能less写代码,让编译器在适当的时候清理这些东西。
我在C ++代码中使用goto的唯一两个原因是:
- 打破一个级别2+嵌套循环
-
像这样一个复杂的stream程(在我的程序中的评论):
/* Analysis algorithm: 1. if classData [exporter] [classDef with name 'className'] exists, return it, else 2. if project/target_codename/temp/classmeta/className.xml exist, parse it and go back to 1 as it will succeed. 3. if that file don't exists, generate it via haxe -xml, and go back to 1 as it will succeed. */
对于代码的可读性,在这个评论之后,我定义了step1标签,并在步骤2和3中使用它。实际上,在60多个源文件中,只有这种情况和一个4级嵌套是我使用的地方。 只有两个地方。
以上所有内容都是有效的,您可能也想看看是否可以降低代码的复杂度,并通过减less标记为“大量代码”部分中的代码的总数来减less对goto的需求在你的例子。 附加delete 0
是一个有效的C ++语句
Using GOTO labels in C++ is a bad way to program, you can reduce the need by doing OO programming (deconstructors!) and trying to keep procedures as small as possible.
Your example looks a bit weird, there is no need to delete a NULL pointer . And nowadays an exception is thrown when a pointer can't get allocated.
Your procedure could just be wrote like:
bool foo() { bool bRetVal = false; int p = 0; // Calls to various methods that do algorithms on the p integer // and give a return value back to this procedure. return bRetVal; }
You should place a try catch block in the main program handling out of memory problems that informs the user about the lack of memory, which is very rare … (Doesn't the OS itself inform about this too?)
Also note that there is not always the need to use a pointer , they are only useful for dynamic things . (Creating one thing inside a method not depending on input from anywhere isn't really dynamic)
I am not going to say that goto
is always bad, but your use of it most certainly is. That kind of "cleanup sections" was pretty common in early 1990's, but using it for new code is pure evil.
The easiest way to avoid what you are doing here is to put all of this cleanup into some kind of simple structure and create an instance of it. For example instead of:
void MyClass::myFunction() { A* a = new A; B* b = new B; C* c = new C; StartSomeBackgroundTask(); MaybeBeginAnUndoBlockToo(); if ( ... ) { goto Exit; } if ( ... ) { .. } else { ... // what happens if this throws an exception??? too bad... goto Exit; } Exit: delete a; delete b; delete c; StopMyBackgroundTask(); EndMyUndoBlock(); }
you should rather do this cleanup in some way like:
struct MyFunctionResourceGuard { MyFunctionResourceGuard( MyClass& owner ) : m_owner( owner ) , _a( new A ) , _b( new B ) , _c( new C ) { m_owner.StartSomeBackgroundTask(); m_owner.MaybeBeginAnUndoBlockToo(); } ~MyFunctionResourceGuard() { m_owner.StopMyBackgroundTask(); m_owner.EndMyUndoBlock(); } std::auto_ptr<A> _a; std::auto_ptr<B> _b; std::auto_ptr<C> _c; }; void MyClass::myFunction() { MyFunctionResourceGuard guard( *this ); if ( ... ) { return; } if ( ... ) { .. } else { ... } }
A few years ago I came up with a pseudo-idiom that avoids goto, and is vaguely similar to doing exception handling in C. It has been probably already invented by someone else so I guess I "discovered it independently" 🙂
BOOL foo() { BOOL bRetVal = FALSE; int *p=NULL; do { p = new int; if(p==NULL) { cout<<" OOM \n"; break; } // Lot of code... bRetVal = TRUE; } while (false); if(p) { delete p; p= NULL; } return bRetVal; }
I think using the goto for exit code is bad since there's a lot of other solutions with low overhead such as having an exit function and returning the exit function value when needed. Typically in member functions though, this shouldn't be needed, otherwise this could be indication that there's a bit too much code bloat happening.
Typically, the only exception I make of the "no goto" rule when programming is when breaking out of nested loops to a specific level, which I've only ran into the need to do when working on mathematical programming.
例如:
for(int i_index = start_index; i_index >= 0; --i_index) { for(int j_index = start_index; j_index >=0; --j_index) for(int k_index = start_index; k_index >= 0; --k_index) if(my_condition) goto BREAK_NESTED_LOOP_j_index; BREAK_NESTED_LOOP_j_index:; }
That code has a bunch of problems, most of which were pointed out already, for example:
-
The function is too long; refactoring out some code into separate functions might help.
-
Using pointers when normal instances will probably work just fine.
-
Not taking advantage of STL types such as auto_ptr
-
Incorrectly checking for errors, and not catching exceptions. (I would argue that checking for OOM is pointless on the vast majority of platforms, since if you run out of memory you have bigger problems than your software can fix, unless you are writing the OS itself)
I have never needed a goto, and I've always found that using goto is a symptom of a bigger set of problems. Your case appears to be no exception.
Using "GOTO" will change the "logics" of a program and how you enterpret or how you would imagine it would work.
Avoiding GOTO-commands have always worked for me so guess when you think you might need it, all you maybe need is a re-design.
However, if we look at this on an Assmebly-level, jusing "jump" is like using GOTO and that's used all the time, BUT, in Assembly you can clear out, what you know you have on the stack and other registers before you pass on.
So, when using GOTO, i'd make sure the software would "appear" as the co-coders would enterpret, GOTO will have an "bad" effect on your software imho.
So this is more an explenation to why not to use GOTO and not a solution for a replacement, because that is VERY much up to how everything else is built.
I may have missed something: you jump to the label Exit if P is null, then test to see if it's not null (which it's not) to see if you need to delete it (which isn't necessary because it was never allocated in the first place).
The if/goto won't, and doesn't need to delete p. Replacing the goto with a return false would have the same effect (and then you could remove the Exit label).
The only places I know where goto's are useful are buried deep in nasty parsers (or lexical analyzers), and in faking out state machines (buried in a mass of CPP macros). In those two cases they've been used to make very twisted logic simpler, but that is very rare.
Functions (A calls A'), Try/Catches and setjmp/longjmps are all nicer ways of avoiding a difficult syntax problem.
保罗。