4
\$\begingroup\$

How to make this code better?

if (Program.Data.DataBase.TryOpenDataBase())
{
 bool result; 
 DataBase= new Program.Data.DataBase(out result);
 if (!result)
 {
    if (Program.DataBase!= null) 
       Program.DataBase.Dispose();
     Program.DataBase= null;
     Log.WriteERROR("Can not open database");
     Application.Exit();
     return;
 }
 else
 {
    Log.WriteERROR("Can not open database");
    Application.Exit();
    return;
 }
\$\endgroup\$
1
  • 1
    \$\begingroup\$ There's a lot of stuff here that appears to be proprietary and has no real meaning in a usable context. Are you using MSSQL, MySQL, Oracle? What does TryOpenDataBase() look like? What is DataBase? \$\endgroup\$ May 5, 2011 at 14:22

2 Answers 2

4
\$\begingroup\$

I would remove the out parameter from the constructor. It is really confusing to read. Maybe create a function that throws an exception instead. If the database implements IDisposable you can use a using block instead.

Edit - You have some duplicate code as well:

Log.WriteERROR("Can not open database");
Application.Exit();
return;
\$\endgroup\$
1
\$\begingroup\$

Generally you shouldn't return any error codes, use exceptions and try/catch blocks.

\$\endgroup\$
1
  • \$\begingroup\$ To be clear, returning the error code doesn't just have be the return value of the method, it could be an out parameter. This is still usually a bad thing. \$\endgroup\$
    – mjcopple
    May 26, 2011 at 20:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.