2

I have a function called connect like so:

public boolean connnect(){
{
 ..... connecting codde
 if(connectionSuccessfull)
 {
   return true;
 }
  else
 {
   return false;
 }
}

This is a very basic form of error handling, I want to upgrade this function to handle errors correctly. Such as not just tell me false there was an error, but be able to say, error, Authentication failed, or Time-out error etc. This information then needs to be sent back up the line to the Caller so it can know what happened.

What is the correct way to go about doing this?

{EDIT} In my care its quite probable that an exception will occur I would say 50% of the time.

I have come up with this, does it look partially correct?

namespace MobileWebServices.Exceptions
{
    //Timeout
public abstract class TimeOutException : Exception
{

}

public class ConnectingTimeOutException : TimeoutException
{
}

public class DissconnectingTimeOutException : TimeoutException
{

}

//Authetntication
public abstract class AuthenticationException : Exception
{

}
public class BadAuthenticationException : AuthenticationException
{

}
}
Zapnologica
  • 22,170
  • 44
  • 158
  • 253

4 Answers4

2

The normal approach is to throw an exception (perhaps of a user-defined type), and then to catch those exceptions at a higher level.

If for some reason you cannot use exceptions, you could instead write a wrapper class that encompassed an error message (which would be null if no error occurred) and the bool result (which would only be relevant if the error message is null).

However, I would recommend using exceptions. (The only issue might be whether or not you need to globalise the error message string in the exception, but the consensus is that you should not.)

Community
  • 1
  • 1
Matthew Watson
  • 104,400
  • 10
  • 158
  • 276
  • Oh ok I see, So I can return a object called Error that I define. In what cases wouldn't I use exception? Are try Catches not slower? I could be mistaken. – Zapnologica May 20 '14 at 11:37
  • 1
    This is good and common, but instead of bool consider using int for status (0 for OK, rest for ERROR/FAILURE), so you don't need to parse strings if you wish to react specifically. – hauron May 20 '14 at 11:37
  • 1
    @Zapnologica Yes try/catch has a large overhead if an exception is actually thrown. However, if you expect that exceptions should not generally be thrown, then they are the correct approach. And also consider that in the context of I/O failures, exception handling is normally very quick compared to the actual I/O speed. – Matthew Watson May 20 '14 at 11:39
  • @hauron Or instead of `int` for status, use `enum` so you have something meaningful instead of just numbers? – crashmstr May 20 '14 at 11:40
  • @hauron Error codes can be useful, but they can also be very brittle. If the comms code is in a library and you need to add some new error codes, it means that you effectively break the interface and all the client code must change to handle the new error codes. (Unless you just show the error code number to the user, which is I suppose an option.) – Matthew Watson May 20 '14 at 11:41
  • `try`-`catches` are slower when the exception happens, but they save a lot of code in the more common path where you're otherwise constantly checking if the last thing attempted worked (or forgetting to check, and having hard to trace bugs). It also is generally the case where the speed is less important (e.g. if I press a button on a desktop app that downloads something, I'd rather it downloaded information for me quickly when it works, rather than be a few nanoseconds faster in showing me an error dialog in the exception handling). In all using exceptions speeds up much code. – Jon Hanna May 20 '14 at 11:49
  • 1
    @MatthewWatson You may have a string describing the error/status. If you want to pass just a single value then indeed the user won't learn much, but hiding the internals of the problem may be the way to go. – hauron May 20 '14 at 11:52
2

Something along the lines of:

public void Connect()
{
  try
  {
    //code here to look-up the connection details
    if(!ValidateConnectionDetails(details))
      throw new InvalidOperationException("The connection details are not valid.");
    //code here to establish the connection
    if(SomeTestThatShowsWereNotHappyWithTheConnection())
      throw new Exception("The connection is bad, for some reason");
  }
  catch(SocketException se)
  {
    //We'd only have this block if a socket exception is possible. We might just allow it to pass on up.
      throw; // User now gets the exception we got, exactly.
    //We might re-throw the error, but from here so the stack-trace goes to here rather than the innards of this method:
      throw se;
    //Most usefully we might throw a new exception that contains this as an inner exception:
      throw new Exception("Connecting failed", se);
    //Or even better, we might throw a more well-defined exception, that relates to this operation more specifically, with or without the inner exception, depending on whether that is likely to be useful:
      throw new ConnectionException("Some message, or maybe just a default is defined in the constructor");
    //OR:
      throw new ConnectionException("Some message, or maybe just a default is defined in the constructor", se);
  }
  catch(Exception ex)
  {
    //If we get to an exception ourselves that isn't of a particular type we're expecting, we probably shouldn't catch it at all. We might though want to note the exception before re-throwing it, or throw a more specific connection with this as an inner-exception:
    Log(ex);
    throw;
  }
}

Because you're no longer returning a value to indicate success, you could also now return an object that represents the connection you created:

public ConnectionObject Connect()
{
   // Much as above, but returning the object before the end of the `try`.
}

Returning values representing failure should only be done if that failure is both likely to happen, and something you expect the calling code to be able to reasonably react to right at the point of calling. This isn't that likely with code to connect since the calling code could be code that e.g. connects and then does an operation, and the code calling that in turn is where the exception (whether from here or the subsequent operation) should be caught - it's the code that ultimately cares about the failing.

In the latter case, then returning a value indicating the failure makes a lot more sense. Here though, I'd probably still consider an exception, because it can encapsulate more information, be used by coders in the normal way they use other .NET methods, and because the calling code is probably not written thinking "try to get the connection and then if it works..." it's written thinking "get the connection and then..." with the error case being exactly that; an error case. (For comparison, a method like int.TryParse() is to answer the question "does this string represent an integer, and if so what is it?" where the method int.Parse() answers the question "what is the integer in this string?" with there not being an integer being an error condition).

To think of it another way. Are you currently using a web-browser to browse the web, or are you using it to try to browse the web? Your internet connection could die on you, stopping you from continuing to read these answers, but you'd consider that a problem in what you were trying to do.

Jon Hanna
  • 110,372
  • 10
  • 146
  • 251
1

Here is a sample on how things should be done :
First use your connect() method to return an object (like a Socket for example).
Return a null one if it fails connecting without throwing an Exception.
In your connect() method try/catch your connecting instructions, and rethrow those catched.
Then in the calling method, catch all the Exceptions that can be thrown, and check if the returned object is null or not.
Here is an example of code using Sockets :

public static Socket connect()
{
    Socket s = null;
    try
    {
        IPEndPoint iEP = new IPEndPoint("127.0.0.1", 8080);
        s = new Socket(iEP.AddressFamily, SocketType.Stream, ProtocolType.Tcp);
        s.Connect(iEP);
        if(!s.Connected)
        {
            return null;
        }
    }
    catch(Exception e)
    {
        throw e;// Rethrow the Exception to the caller
    }
    return s;
}

public static void Main(String[] args)
{
    Socket mySocket = null;
    try
    {
        mySocket = connect();
    }
    catch(SocketException e)
    {
        // TODO - Detailed error about a SocketException
        Console.Error.WriteLine("SocketException: " + e.Message + "(" + e.ErrorCode + ")");
    }
    catch(SecurityException e)
    {
        // TODO - Detailed error about a SecurityException
        Console.Error.WriteLine("SecurityException: " + e.Message);
    }
    catch(Exception e)
    {
        // TODO - Detailed error about those Exceptions :
        // ArgumentNullException, ObjectDisposedException and InvalidOperationException
        Console.Error.WriteLine(e.GetType() + ": " + e.Message);
    }

    if(mySocket == null)
    {
        // TODO - Error while initializing the Socket
        Console.Error.WriteLine("Error while initializing the Socket");
    }

    // TODO - Use your Socket here
}
Hybris95
  • 2,286
  • 2
  • 16
  • 33
  • Why do you *rethrow*? Isn't it enough to do not use `try`/`catch` in `connect()` at all? – Sinatr May 20 '14 at 11:54
  • If you have more things to do in a finally block for example closing a connection or releasing resources, undoing some tasks. But essentially, catching the Exceptions in the calling method is the main thing to do. – Hybris95 May 20 '14 at 11:57
  • I like this example, The code im calling in the connect function does throw any exceptions, I will be checking for certain cases and then throwing the error. How do I create im own exceptions? because they obviously need to be relative to my application? – Zapnologica May 20 '14 at 11:57
  • You can make your own Exception class and then throw it with `throw new MyException("Error description");` – Hybris95 May 20 '14 at 12:00
  • For example..the hardest part will be to detect WHEN you will throw this or this Exception. By the way, standard exception already thrown (like SocketException for example) give this kind of details. EXAMPLE : `catch(SocketException e){if(e.ErrorCode == 10051){ throw new NetworkUnreachableException(); }}` You can find details on these error codes here (http://msdn.microsoft.com/en-us/library/windows/desktop/ms740668%28v=vs.85%29.aspx) – Hybris95 May 20 '14 at 13:37
0

I think the best way is using the try catch exception surrounding your call with the exception you want :

catch(TimeoutException ex)
{
//Do something
}
catch(SqlException ex)
{
//do something
}

//....

catch(Exception ex)
{
//do something
}

Make sure of the order of your catch ( the global Exception in last)

Alexein1
  • 109
  • 1
  • 6