Exception handling?
in [MFC]
|
Prev: Deriving from a class derived from a CDialog
Next: How to show underlined menu shortcut letters by default for a contextmenu ?
From: Giovanni Dicanio on 22 Jun 2010 18:18 On 22/06/2010 23:06, David Ching wrote: > Exceptions are meant for rarely occurring ERROR conditions (you > know, 'exceptional' conditions!), not normal control flow.I once saw a > switch statement rewritten as a bunch of thrown exceptions, not a pretty > sight. I do agree with you David. The Parse vs. TryParse case of .NET comes in my mind: the earlier versions of the framework had the Parse method, which threw exceptions in case of parsing error. In later versions (since 2.0?) they introduced TryParse, which just returned an error code. Surrounding Parse calls with try/catch blocks produced bad code; instead a simple if-check of TryParse return code is much better, IMHO. Giovanni
From: RB on 22 Jun 2010 18:35 > *** > But the point here is that it *has* called the virtual method > ReportSaveLoadException. and it return FALSE from OnOpenDocument. > ............ > Because this is a virtual method, you could override it and if calling the parent > returned NULL, you could do something useful such as setting the filename to > empty. I think this code actually is incorrect, but you can create your own > subclass and do whatever you want. ------------------------------------------ Hey Joe, this is getting a bit confusing for me. Why would I want to create my own subclass to set the filename to empty when the below framwork code (that I previously posted ) sets the filename to "untitled" for me which keeps me from inadvertly saving and overwriting any previous persistence ? ? >> if (!pDocument->OnOpenDocument(lpszPathName)) >> { >> ........ >> else >> { >> SetDefaultTitle(pDocument); >>// RB the above sets the filename to "untitled" >>...........
From: Doug Harrison [MVP] on 22 Jun 2010 18:43 On Tue, 22 Jun 2010 17:27:17 -0400, Joseph M. Newcomer <newcomer(a)flounder.com> wrote: >Part of the problem is that not everything is encapsulable in RAII. For example, if I >should return a pointer to an object, the object must have a lifetime that exceeds scope. >But if there is a failure, I should free the storage up. RAII doesn't handle this >gracefully. Other examples include "either I will return a handle or no handle will be >allocated" but you have to allocate the handle to make progress. RAII is powerful, >partcularly with smart pointers, but it is always based on the premise that leaving scope >means deletion of whatever resources are consumed, and that is not always feasible. That's really not a problem. Smart pointer classes can have a "release" function, that returns the encapsulated pointer and relinquishes ownership. For example, here's a function I wrote to get an IShellFolder interface (encapsulated in a COM smart pointer) and optionally return a PIDL for an absolute path: IShellFolderPtr GetFolder( const wchar_t* path, LPITEMIDLIST* pidlFolder = 0, HWND hWnd = 0) { IShellFolder* pDesktop = GetDesktop(); nc_ITEMIDLIST_ptr pidl(GetPidl(pDesktop, path, hWnd)); IShellFolder* pFolder; Co::Vet(pDesktop->BindToObject( pidl.get(), 0, IID_IShellFolder, (void**) &pFolder)); if (pidlFolder) *pidlFolder = pidl.release(); return IShellFolderPtr(pFolder, false); } I used the nc_ITEMIDLIST_ptr ("nc" means non-copyable i.e. not reference-counted and not weirdly copyable like auto_ptr) to protect the PIDL across the call to Co::Vet, which is my function to turn HRESULT errors into exceptions. After the IShellFolder pointer is obtained, if pidlFolder is non-NULL, the pointer is released from pidl and stored to it, transferring ownership to the caller. If pidlFolder is NULL, the PIDL is deleted when the function returns and pidl is destroyed. The creation of the IShellFolderPtr cannot fail, so this is exception-safe. (The GetDesktop result is a cached value that is allocated on first call, so it doesn't leak if an exception is thrown.) Using RAII like this can linearize a function that would otherwise be cluttered with multiple try/catch blocks or deeply nested ifs or serial ifs with multiple goto targets at the bottom of the function. -- Doug Harrison Visual C++ MVP
From: David Ching on 22 Jun 2010 18:58 "RB" <NoMail(a)NoSpam> wrote in message news:#h3PhelELHA.1272(a)TK2MSFTNGP05.phx.gbl... > Well I'm glad that my thread is getting a lot of input, but > most of the talk on this section is loosing me at my level. > I'm getting confused as to whether I should "try and catch" > on not "try and catch". If you would, could you please comment on my code > with question comments below. > It is not that long and you can just say whatever brief > or elongation review you have time for. > --------------- > Additionally I have this logic going so far. > // in the read loop of MyDoc class serialize > else { try > { ar >> FID_Read;; // DWORD FID_Read; if > (FID_Read != FileID) // const DWORD FileID; > { // FileID mismatch throw new CWrongFileIDException(); } > ar >> VerData.Ver >> VerData.CpyRt >> VerData.Corp; > ar.SerializeClass(RUNTIME_CLASS(CMapStringToString)); > ExpMap1.Serialize(ar); } > catch(CWrongFileIDException* e) { > // Do whatever I may need here > // so far nothing that I can tell, in fact it seems I could have > just > // eliminated my try and catch handlers and just called the > // AfxThrowArchiveException in the if loop above ? > e->Delete( ); > AfxThrowArchiveException(CArchiveException::badIndex, NULL ); > //Invalid file format > // The above takes me to the exact same cleanup code that I get when > // I read in a corrupt file (with NO exception code at all ) and mfc > handlers it. > // And leaves me with an untitled filename eliminating the change if > inadvertant > // save overwrite. > } > } > I agree with your final comment; there is no reason to throw CWrongFileIDException when the only thing catching it is right below. Optimize like this: ar >> FID_Read;; // DWORD FID_Read; if (FID_Read != FileID) // const DWORD FileID; { // FileID mismatch AfxThrowArchiveException(CArchiveException::badIndex, NULL ); //Invalid file format return; } ar >> VerData.Ver >> VerData.CpyRt >> VerData.Corp; ar.SerializeClass(RUNTIME_CLASS(CMapStringToString)); ExpMap1.Serialize(ar); -- David
From: Joseph M. Newcomer on 22 Jun 2010 19:49
See below... On Tue, 22 Jun 2010 18:09:39 -0400, "RB" <NoMail(a)NoSpam> wrote: >Well I'm glad that my thread is getting a lot of input, but >most of the talk on this section is loosing me at my level. >I'm getting confused as to whether I should "try and catch" >on not "try and catch". If you would, could you please >comment on my code with question comments below. >It is not that long and you can just say whatever brief >or elongation review you have time for. >--------------- > Additionally I have this logic going so far. >// in the read loop of MyDoc class serialize > else > { > try > { > ar >> FID_Read;; // DWORD FID_Read; > if (FID_Read != FileID) // const DWORD FileID; > { // FileID mismatch > throw new CWrongFileIDException(); **** As already pointed out, doing a throw directly inside a try is probably not good style. Now if you called another function that called a function that called a function that did a throw, that makes more sense. Key here is how much you have to do in the catch. Putting all the recovery code in the catch would consolidate it and eliminate that horror of "goto exit" that happens so often. But frankly, if you're doing that, the right way to handle it is more likely to put the code in a function this one calls, and just return FALSE if there is an error. Then do what you need to do to clean up in the else branch of the if. **** > } > ar >> VerData.Ver >> VerData.CpyRt >> VerData.Corp; > ar.SerializeClass(RUNTIME_CLASS(CMapStringToString)); > ExpMap1.Serialize(ar); > } > catch(CWrongFileIDException* e) > { > // Do whatever I may need here > // so far nothing that I can tell, in fact it seems I could have just > // eliminated my try and catch handlers and just called the > // AfxThrowArchiveException in the if loop above ? > e->Delete( ); > AfxThrowArchiveException(CArchiveException::badIndex, NULL ); //Invalid file format >// The above takes me to the exact same cleanup code that I get when >// I read in a corrupt file (with NO exception code at all ) and mfc handlers it. >// And leaves me with an untitled filename eliminating the change if inadvertant >// save overwrite. > } > } Joseph M. Newcomer [MVP] email: newcomer(a)flounder.com Web: http://www.flounder.com MVP Tips: http://www.flounder.com/mvp_tips.htm |