On Mon, May 12, 2014 at 09:50:06PM -0300, Tomaz Canabrava wrote: > > So I changed the silly C version of Q_ASSERT to at least tell us what the > > offending id was and to print to stderr. But I'm not sure I agree with you > > on removing the if (!dive) checks - if compiled without DEBUG this means > > we will dereference NULL pointers if for some reason there's a stale id > > somewhere. > > Yes, it will. and the app will crash. the Q_ASSERT ( only in debug > mode, of course > ) will also make the app crash right away. the idea of an assert is > that: stop executing > the application right away to so the developer can fix the problem.
yes, of course. > The ID thing, we are not making up id's for the application, we are > getting dives that > gives us ID's, and using it. *if* we are forgetting to clean that id > after a dive is deleted , > removed or anything, it's an application bug and a crash right away is > a good way to > let is track it down, instead of masking it with a early return of a > function that should > print something, but notthing appears. I disagree. Crashing in a release build is NOT a good way to let the user know that something is wrong. We should avoid crashes as much as humanly possible. > > I would have to go back through the commit history to figure out if these > > were spots were this could conceivably happen - but in general I'm not > > sure I like the idea that we simply crash if we get an unexpected error > > somewhere. I'd rather figure out ways to keep going (which is what the old > > code did). > > > > Can you comment on that, Tomaz? > > > > I took the patch and just pushed it - so we may have to go in and add the > > null-ness checks back in. > > See if my last comment makes sense :) > I think it does, but I'm a weird one ;p Yes, you are. I much rather have us recover gracefully and tell the user in a message window "hey, you ran into a situation that shouldn't happen, here is some info on what happened, don't worry if this doesn't make sense, just open a bug report at trac.hohndel.org, cut and paste this info an we'll take a look. for now you can just keep going." and then print out what happened where and whatever else in useful debug info we can give ourselves. /D _______________________________________________ subsurface mailing list [email protected] http://lists.hohndel.org/cgi-bin/mailman/listinfo/subsurface
