//========================================================================
//TITLE:
// 如何写优雅的代码(1)——灵活使用goto和__try:评论反馈
//AUTHOR:
// norains
//DATE:
// Tuesday 21-July-2009
//Environment:
// WINCE5.0 + VS2005
//========================================================================
本文会引起不少的争议,也算意料之中,毕竟文无定法。不过,有些朋友的观点我确实也考虑过,但并不是十分赞同。又因为直接回复格式实在无法统一,故在此撰文一并作答。
1.feelapi 发表于2009年7月16日 17:48:48 IP:举报
VC8.0以后就不用担心SEH冲突的问题,都统一了,可以使用/EHa编译选项,就没问题了
我用vs2005测试如下代码,还是不行:
int WINAPI WinMain( HINSTANCE hInstance,
HINSTANCE hPrevInstance,
LPTSTR lpCmdLine,
int nCmdShow)
{
__try
{
std::vector<BYTE>::iterator iter;
}
__finally
{}
return 0;
}
我设置的方式如下:property-->c/c++--->Code Generation--->Enable C++ Exceptions,选择:yes with SEH exceptions(/EHa)。
编译出错信息:Cannot use __try in functions that require object unwinding
不知道是不是我设置的问题?
2.titilima 发表于2009年7月17日 11:41:01 IP:举报
do-while (0) 可以更优雅地解决这个问题,而且不会带来 SEH 的额外开销。
xylophone21 发表于2009年7月20日 12:55:49 IP:举报
do { break; }while(0)
jack1003 发表于2009年7月18日 9:45:02 IP:举报
探讨下:楼主这样实现也可以,但用TRY 总觉得有点费解,看看这样实现怎么样?
BOOL ReadDeviceBuf()
{
EnterCriticalSection(&g_csBuf);
bool bOk = true;
do { //打开驱动设备 HANDLE hDev = CreateFile(TEXT("DEV1:"), FILE_WRITE_ATTRIBUTES, 0, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL|FILE_FLAG_WRITE_THROUGH, NULL);
if(hDev == INVALID_HANDLE_VALUE)
{ bOk = false; break; } //获取驱动设备的缓存大小 DWORD dwSize = 0; if(DeviceIoControl(hDev,IOCTRL_BUFFER_SIZE,NULL,0,&dwSize,sizeof(dwSize),NULL,NULL) == FALSE)
{ bOk = false; break; } //分配缓存 g_pBuf = new BYTE[dwSize]; if(g_pBuf == NULL) { bOk = false; break; } //从驱动中读取数据 if(DeviceIoControl(hDev,IOCTRL_GET_BUFFER,NULL,0,&g_pBuf,dwSize,NULL,NULL) == FALSE)
{
delete []g_pBuf;
g_pBuf = FALSE;
bOk = false;
break;
}
} while (false);
CloseHandle(hDev);
LeaveCriticalSection(&g_csBuf);
return bOk ? TRUE:FALSE;
}
这样确实也可以,效率也比__try高,唯一的瑕疵是“词不达意”——毕竟while这关键字是用来表达循环的。这也是我为什么不选用while的原因,仅仅是个人喜好而已。
3.kinsan 发表于2009年7月17日 16:15:43 IP:举报
我面试人 只要有程序里写goto的 一律刷掉 goto 不容易掌握流程 程序规模越大就越难控制 就你那么几行代码 能说明什么? 世人鄙弃goto 不是没有理由的
看来微软wince团队的那些巨擘想进你们公司都不容易啊,因为wince的源代码中有太多的goto出自于这些牛人之手了...所以齐刷刷被你刷掉了... :-) 你可以查看一下wince5.0的public部分代码,一共有5086处用到了goto,这还没算private等其它的文件夹....
还有一些网友对此的看法:
kryso 发表于2009年7月17日 18:01:31 IP:举报
只能说你的教条主义严重.不容易掌握流程是代码编写能力的问题,证明你的水平没到家.
jzkdl 发表于2009年7月18日 15:00:29 IP:举报
我认为goto本身没有错~~ 对于goto来说更适合用在编写较底层的驱动代码~~或者简单函数里面~毕竟goto本身就是汇编JMP指令的代表~用goto有时候更容易理解计算机的运行原理~~ 试想一下,现在各种系统的内核不是还使用汇编吗~~ 如果你硬要拿goto和面向对象一起讨论~~我不发表意见~ 我个人非常支持作者的观点~~用goto只是为了优雅~~ 这里不是讨论到底应不应该用goto~~
wlzqi 发表于2009年7月18日 22:04:09 IP:举报
"我面试人 只要有程序里写goto的 一律刷掉 goto 不容易掌握流程 程序规模越大就越难控制 就你那么几行代码 能说明什么? 世人鄙弃goto 不是没有理由的" ---------你还太浅了。
ayave 发表于2009年7月18日 22:09:35 IP:举报
“我面试人 只要有程序里写goto的 一律刷掉 goto 不容易掌握流程 程序规模越大就越难控制 就你那么几行代码 能说明什么? 世人鄙弃goto 不是没有理由的” 哪个说的世人摒弃goto,linux内核满篇的goto, 我不认为linux内核就是垃圾。
cppmobile 发表于2009年7月19日 23:05:57 IP:举报
不需要盲目的去鄙弃一个东西,只要用得其所就可以用。
4.morphia 发表于2009年7月18日 7:07:55 IP:举报
为了个C 对象就改用goto!!非常糟糕! 真正的解决办法:把你的那些HANDLE用对象封装起来,像std::auto_ptr一样,然后还有你的与CriticalSection的东东都封装起来,在对象destroy的时候释放资源,使用_set_se_translator将SEH的异常封装成对象抛出,使用try catch处理各种不同类型的异常,才叫完美解决。 你这种解决办法,一两百行代码还行,代码多了你就完蛋了。
说实话,函数的目的就是精简。如果函数代码超过一两百行,再不分割为各自小函数的话,估计就是思路有问题了。
5.morphia 发表于2009年7月18日 7:26:07 IP:举报
另外补充,try catch应该在该函数之外捕获异常,而当你的设备打开,读取,确定大小失败的时候就应该抛置你自己定义的异常,这时函数直接因为异常而退出,而因为你的HANDLE和CRITICALSECTION被对象封装,并且在对象释放的时候会自动释放HANDLE和LEAVE CRITICALSECTION,所以异常不会导致资源没有被释放。具体的,参见《More Effective C 》里有关章节。
cplusplus_zk 发表于2009年7月18日 13:54:29 IP:举报
俺认为,像里面那个临界区的处理,还是封装成一个类更优雅。用goto我可以容忍,但绝对不应该推荐,特别是很多C 并不是很精通的程序员很容易收到误导的。
barsteng 发表于2009年7月20日 17:13:49 IP:举报
同意morphia和cplusplus_zk 的观点,不知道楼主是否会考虑用类封装资源的管理更优雅?而且更容易被C developer理解我post了一个实现,可惜格式缩进什么的都被过滤了,呵呵
用类来做互斥量的确是个好办法,比如:
class CLock
{
public:
CLock(LPCRITICAL_SECTION pcsLock):
m_pcsLock(pcsLock)
{
EnterCriticalSection(m_pcsLock);
};
virtual ~CLock()
{
LeaveCriticalSection(m_pcsLock);
};
private:
LPCRITICAL_SECTION m_pcsLock;
};
调用的时候:
BOOL ReadDeviceBuf()
{
CLock lock(&g_csBuf);
...
}
这在当前函数例子中表现的确很好,也在大部分情况下很优雅。但如果函数需要多次进入临界点,那么就不显得那么美好了,如:
static DWORD MonitorProc(LPVOID pParam)
{
CLock lock1(&g_csBuf);
//做一些事情
~lock1(); //显式调用析构函数
//做一些事情,比如通知别的进程数据有改变
CLock lock2(&g_csBuf);
//做一些事情
~lock2(); //显式调用析构函数
}
姑且不说调用析构函数和直接调用LeaveCriticalSection无异,就说显式调用析构函数,也属于极少使用的技法。这是这个方法的唯一瑕疵。当然,我举的这个反例或多或少是有点吹毛求疵了。
6. Analyst 发表于2009年7月20日 21:45:25 IP:举报
不管是用goto还是__try __finally都是糟糕的C 写法,只有C程序才会这么写。 正确的C 写法是这样的:
void ReadDeviceBuf()
{
HANDLE hDev;
try{
CriticalSectionGuard lock((&g_csBuf);
hDev = CreateFile(TEXT("DEV1:"), FILE_WRITE_ATTRIBUTES, 0, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL|FILE_FLAG_WRITE_THROUGH, NULL);
if(hDev == INVALID_HANDLE_VALUE)
throw SysException(GetLastError());
DWORD dwSize = 0;
if(DeviceIoControl(hDev,IOCTRL_BUFFER_SIZE,NULL,0,&dwSize,sizeof(dwSize),NULL,NULL) == FALSE)
throw SysException(GetLastError());
//....
CloseHandle(hDev);
}
catch(SysException& e)
{
CloseHandle(hDev);
throw e;
}
}
这是一个看起来很美的写法,但实际上还是有问题的。
写代码,有一个原则是:Input once,output once. 翻译成中文,也就是一进一出相对应。而这代码偏偏违反了这个原则。
为了方便,我在代码加上标签:
void ReadDeviceBuf()
{
HANDLE hDev;
try{
CriticalSectionGuard lock((&g_csBuf);
hDev = CreateFile(TEXT("DEV1:"), FILE_WRITE_ATTRIBUTES, 0, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL|FILE_FLAG_WRITE_THROUGH, NULL);
if(hDev == INVALID_HANDLE_VALUE)
throw SysException(GetLastError());
DWORD dwSize = 0;
if(DeviceIoControl(hDev,IOCTRL_BUFFER_SIZE,NULL,0,&dwSize,sizeof(dwSize),NULL,NULL) == FALSE)
throw SysException(GetLastError());
//....
A:
CloseHandle(hDev);
}
catch(SysException& e)
{
B:
CloseHandle(hDev);
throw e;
}
}
如标签A和B所示,这两处地方都属于同义资源释放。换句话说,这两部分的代码基本上是一模一样的,否则就达不到释放资源的目的。
那么很显然,会出现这两个问题:
1).如果释放资源的代码行数比较多,而A和B必须是同样的代码,由此造成函数的体积膨胀加快,显得极为臃肿。
例如:
void ReadDeviceBuf()
{
//....
A:
delete [] pBuf1;
delete [] pBuf2;
NotifyError();
CloseHandle(hDev);
}
catch(SysException& e)
{
B:
delete [] pBuf1;
delete [] pBuf2;
NotifyError();
CloseHandle(hDev);
throw e;
}
}
2).当代码持续膨胀,后续的修改者又不是作者本人的时候,很难保证标签A和B的代码一致。如果不一致,甚至只是少些一两段语句,那么肯定会造成资源泄漏。如果长时间运行,很有可能有异常发生。
另外,更正一点,__try __finaly并不是什么糟糕的C写法,这个是SEH,由windows所特有支持的机制(所以不具备移植性)。至于__try __finaly是否是糟糕的做法,我只能说的是,在C#的语言层面已经支持和该机制类似的方法。对于号称去掉C/C++繁琐和难懂等弊端的现代C#语言,也支持该C++所不具备的特性,也许能说明一些问题吧?