Hi,
I am working in a code-base(C# 3.5), which is littered with exception handling blocks which return true/false, when inside in a function which returns 'bool'.
catch (Exception ex) { return false; }
This is not correct practice.I am thinking of logging the exception, and have a local variable(to function) which will be initialized.And, this variable will be returned at the end of function.
What do you think?
It is highly subjective what you want here.
It is old C-style code to have your functions return a true/false value depending on the function succeding or not. In the C# world, it is alot more common to throw exceptions.
You can debate for a long time which is the right path to choose for your code base.
My general point of view is to stick to the exceptions, they have the advantage that they bubble up and usually require less code. And there is also great support for actually logging uncaught exceptions.
Personally, I would get rid of all of these catch blocks and let the exception bubble up when it happens.
As you say, using exceptions for application logic is bad practice.
Have a catchall exception handler that will catch all such exceptions and log them and terminate the program (if that's the right thing for this program).
The usual accepted way to handle exception is to handle them only if you can do something about it. You can of course handle a generic exception just for log puroposes but you should reraise it once you're done.
Your application logic shouldn't rely on exceptions. If there is absolutely no other way to do it, then at least handle concrete exceptions instead of the generic one...
Although I wouldn't recommend using the bool return value as proper exception handling, if there is already a lot of code written that way, going and making sweeping modifications everywhere may not be the best idea.
Throwing an exception is generally better than returning false - an exception can at least give the receiver some clue of what happened and why the failure occurred.
The return value of a method should be the result of a method, or it should throw an exception explaining why the requested operation could not be completed.
You inherited code where the original programmer just didn't want to deal with errors. As written, the program will simply malfunction (say, not save a record to the database) and there's no way to find out why it malfunctioned. Especially since this is dbase code, this will cause random data corruption and loss of precious data.
You indeed cannot leave the code the way it is. The very first thing you need to do is remove all try/catch statements so the program terminates when there's an error. Add an AppDomain.UnhandledException event handler to log the exception so you know what went wrong. Fix errors you'll encounter by correcting code or validating the data. Do expect this to take time, it is very likely that you'll discover many design problems that were shoved under a doormat before.