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: RB on 21 Jun 2010 18:33 Still working on digesting and attempting to implement all you gave me, but have a couple of quick acknowledgements and questions: > **** > If you want your exception code to clear this variable, you will have to do that > explicitly. Note that if this value is set AFTER the serialization call, because you are > handling the excepiton internally and not throwing it, there is zero hope that you will be > able to deal with this. Instead, you will have to add a handler for operations like > File::Save, you will have to put an exception handler in that code, you will have to have > your very own generic exception class (not CUserException), something like > CMySerializeException, of which CWrongFilieIDException is but one of the many > possible derived classes, and you will have to do a throw; instead of e->Delete() > so your exception is seen outside the serialize handler. Oh ok, well that makes sense. > Never be afraid to create your own classes of exceptions when needed. Never > "hijack" some generic class. Make sure that every exception has useful information in > it so you can make a report (I included file name, line #, and offset-into-line in > CSyntaxException, and for things like CWrongSymbolException I had a placeholder > for the entity I expected, so I would do throw new CWrongSymbolException(file, line, > offset, IDS_EXPECTED_VARIABLE_NAME); Oh, well I am beginning to see more of the concept which I totally misunderstood from the docs I was reading. So I can derive exception classes not necessarily use ones already written. I did find docs on the try and throw. Ugh, do I declare this in global space or in each class it's needed ? class CWrongFileIDException : public CException { public: DWORD GetFileID() { return fid; } CWrongFileIDException(DWORD fileid) { fid = fileid; } protected: DWORD fid; }; ///That's all for now. Thanks !
From: Joseph M. Newcomer on 21 Jun 2010 20:40 See below.... On Mon, 21 Jun 2010 18:33:54 -0400, "RB" <NoMail(a)NoSpam> wrote: >Still working on digesting and attempting to implement all you gave me, but >have a couple of quick acknowledgements and questions: > >> **** >> If you want your exception code to clear this variable, you will have to do that >> explicitly. Note that if this value is set AFTER the serialization call, because you are >> handling the excepiton internally and not throwing it, there is zero hope that you will be >> able to deal with this. Instead, you will have to add a handler for operations like >> File::Save, you will have to put an exception handler in that code, you will have to have >> your very own generic exception class (not CUserException), something like >> CMySerializeException, of which CWrongFilieIDException is but one of the many >> possible derived classes, and you will have to do a throw; instead of e->Delete() >> so your exception is seen outside the serialize handler. > >Oh ok, well that makes sense. > >> Never be afraid to create your own classes of exceptions when needed. Never >> "hijack" some generic class. Make sure that every exception has useful information in >> it so you can make a report (I included file name, line #, and offset-into-line in >> CSyntaxException, and for things like CWrongSymbolException I had a placeholder >> for the entity I expected, so I would do throw new CWrongSymbolException(file, line, >> offset, IDS_EXPECTED_VARIABLE_NAME); > >Oh, well I am beginning to see more of the concept which I totally misunderstood from >the docs I was reading. So I can derive exception classes not necessarily use ones already >written. I did find docs on the try and throw. > >Ugh, do I declare this in global space or in each class it's needed ? > >class CWrongFileIDException : public CException { > public: > DWORD GetFileID() { return fid; } > CWrongFileIDException(DWORD fileid) { fid = fileid; } > protected: > DWORD fid; >}; >///That's all for now. Thanks ! **** It needs to be declared in a context which is known to both the throw and the catch clauses. Typically, I will put this in a header file, e.g., MySerialExceptions.h, and #include it in all the modules that need it. joe **** Joseph M. Newcomer [MVP] email: newcomer(a)flounder.com Web: http://www.flounder.com MVP Tips: http://www.flounder.com/mvp_tips.htm
From: Goran on 22 Jun 2010 04:10 On Jun 21, 6:13 pm, "RB" <NoMail(a)NoSpam> wrote: > In my below foobar first attempt to try exception handling I don't understand > the following aspects: (my code at bottom after these framework pastes) > > 1. I must not be doing something correct because even though both > my CATCH and AND_CATCH call stuff on return that appears to be > deleting the exception and taking care of various cleanup. I would also recommend that you forget TRY, CATCH and the rest of MFC macros. They are a sad leftover from the time MS C++ compiler didn't implement exceptions (but MFC people wanted to have them). MFC has that nasty design flaw with exception handling that it uses exception __pointers__ (I guess also due to said lack of exceptions in compiler in early days). This is not normal in C++ code, and a horrific error. Consequence of these pointers is that design of ownership of said pointers is complicated (well, not much, but still): when you catch an MFC exception, and you don't want to re-throw it, you must call Delete() on it (__NOT__ e.g. delete pException;) I use C++ try/catch and a macro DEL_ON_EXIT, that I put on top of every catch where I want to handle the exception, e.g. like so: try { workworkwork(); } catch(CException* p) { DEL_ON_EXIT(p); p->ReportError(); } DEL_ON_EXIT calls p->Delete() on block exit. Warning: I'll tear apart code below, it's not good at all. ;-) > void CFileHandlingDoc::Serialize(CArchive& ar) > { > if (ar.IsStoring()) > { > ar << FileID; // DWORD > ar << VerData.Ver << VerData.CpyRt << VerData.Corp; > ar.SerializeClass(RUNTIME_CLASS(CMapStringToString)); > ExpMap1.Serialize(ar); > } > else > { > CString E; > TCHAR szErrMsg[255]; > TRY > { > ar >> FileID; > if (FileID != 0x1234ABCD) > { // FileID mismatch > AfxThrowUserException( ); This is bad. CUserException is a special exception type. You use it like so: first, you inform the user what went wrong, then you throw it and leave it to MFC to "handle". MFC actually ignores it, assuming that user was already informed about what went wrong. So in this case, you first inform the user what went wrongm then throw. What happens in this particular case is that it goes to document's ReportSaveLoadException and gets ignored there. So... Given how CUserException is specified, inform the user about what went before throwing. That's rule 0 of CUserException. Rule 1 is: avoid using it. Given that you need to inform the user what's happening, it's better to throw an exception that contains whatever info about the error you want (including message to the user) catch it and report it higher up the stack (most likely, that means letting it be handled by MFC somewhere). There's a catch WRT ReportSaveLoadException and that idea, though, so for now, just inform the user and throw "user" exception. > } > ar >> VerData.Ver >> VerData.CpyRt >> VerData.Corp; > ar.SerializeClass(RUNTIME_CLASS(CMapStringToString)); // WriteClass CmStS's data > ExpMap1.Serialize(ar); // CMapStringToString's keys and values > > } This is bad, and it shows massive confusion WRT exceptions. In 99% of cases, if you throw and catch same exception type in one function, you're doing it wrong. In fact, a piece of advice for a beginner: in 99% of cases, if you think that you need to write a try/catch, you're doing it wrong. What you should do instead is ask yourself "if I throw here, where is exception caught?" Find the answer to that, and you'll find that you don't need a try/catch. (Exception to that rule: various "creation" functions of MFC that you can override, e.g. PreCreateWindow, OnCreate, OnInitDialog, stuff like that especially e.g. OnCreate should not allow that exception escapes from them. > CATCH( CUserException, e ) > { > E = _T("BAD FileID, RB from inside CATCH\n"); > AfxMessageBox( E ); > return; > } > AND_CATCH( CException, e ) > { > // For other exception types, notify user here. > e->GetErrorMessage(szErrMsg, 255); > E = _T("RB from inside AND_CATCH\n"); > E += szErrMsg; > AfxMessageBox( E ); > return; > } > END_CATCH > // No exception thrown. > } > > } In your case, there should not be ANY try/catch statements in the whole of Serialize. What you should do, instead, is let exceptions go through. They will end up in ReportSaveLoadException and be handled there by MFC. Goran. P.S. Don't forget: you are not allowed to write try/catch statements ;-). If you do, you have 99% chance that you're doing it wrong. Good C++ code of today has RIDICULOUSLY small number of try/ catch-es in ti.
From: Goran on 22 Jun 2010 04:15 On Jun 21, 10:33 pm, Joseph M. Newcomer <newco...(a)flounder.com> wrote: > I should also add that the behavior of MFC if an exception is thrown from inside > Serialize() is not documented, and therefore, it does whatever it does. The next release > might do something completely different! Whoa, not true! It's documented in help for CDocument::ReportSaveLoadException. CDocument::ReportSaveLoadException is not perfect (it actually only handles CFile/Archive exceptions, and for the rest it pops (poops, rather) up silly "generic" text ("Failed to open document" I think), so an override of that function might be deemed necessary. It's not undocumented. Goran.
From: RB on 22 Jun 2010 09:45
> This is bad. CUserException is a special exception type. >.......... >........, > it's better to throw an exception that contains whatever info > about the error you want (including message to the user) catch it > and report it higher up the stack (most likely, that means letting it > be handled by MFC somewhere). There's a catch WRT > ReportSaveLoadException and that idea, though, so for now, > just inform the user and throw "user" exception. Ok, ( Joe previously told me I had missed the concept by trying to use another Exception for my specific scenario) so I am understanding that. And even though I not really ready to ask any new questions currently, I did want to respond and let you guys know that you have me on the right track, and I am working on cleaning up my methods while I also learn loads about what is going on and what I need to do in my handler. I've learned so far (from the Debugger) that I don't seem to need to catch any exceptions other than my own. IOW MFC is going to catch a slew of them on it's on and if I interfere with a catch on my end, then I had better know how to forward the throw back into the mfc handler or I am going to lose all of the highly important code thereof. Namely like this Note all my comments are // RB xxxx BOOL CDocument::OnOpenDocument(LPCTSTR lpszPathName) { ....... ....... CATCH_ALL(e) { ReleaseFile(pFile, TRUE); DeleteContents(); // remove failed contents TRY { // RB this (as you already said ) takes me to code // that sorts the exception and reports a msg to // user. ReportSaveLoadException(lpszPathName, e, FALSE, AFX_IDP_FAILED_TO_OPEN_DOC); ........ // RB, then after returning back CArchive code // shuts down the archive and returns back to CDocument* CSingleDocTemplate::OpenDocumentFile( LPCTSTR lpszPathName, BOOL bMakeVisible) { ...... ...... // RB, and comes to this "all important" code which will set the // current document (on return to user ) to "untitled" so that // an inadvertant save does not overwrite any other filename. // I would not get this if I interrupted this exception with my // catch and did not forward it along if (!pDocument->OnOpenDocument(lpszPathName)) { // user has been alerted to what failed in OnOpenDocument TRACE0("CDocument::OnOpenDocument returned FALSE.\n"); if (bCreated) { pFrame->DestroyWindow(); // will destroy document } else if (!pDocument->IsModified()) { // original document is untouched pDocument->SetModifiedFlag(bWasModified); } else { // we corrupted the original document SetDefaultTitle(pDocument); // RB the above is what I was looking for and and now I have // figure out how to call this and any other needed code from any // scenario exception throw of my own that does not finish opening // the document. ............ ............ // RB or otherwise I am going to get this, which // is not good and will effect and inadverntant // overwrite pDocument->SetPathName(lpszPathName); ............. return pDocument; } |