Overloading best practice

Question

I have two static methods that I want to use for error handling. One of which passes the exception object and the other is just used if needing to report an error which would be a text based message (string errorMessage).

The code within the two methods is pretty much the same with the exception of how the message is build up and sent to a log file. How can I refactor this so that I'm not duplicating code?

public static void ReportError(Exception exceptionRaised, string reference, string customMessage, bool sendEmail)
{
    // get filename
    // check if logfile exists, blah, blah
    // build up message from exception, reference & custom message using string builder
    // save message
    // email error (if set)
}

public static void ReportError(string errorMessage, string reference, bool sendEmail)
{
    // get filename
    // check if logfile exists, blah, blah
    // build up message from errorMessage & reference string builder
    // save message
    // email error (if set)
}

Thanks.

Answer 1

Seeing as all you're doing differently is building up a custom message in the first method, change your first method to pass the custom exception through the plain text error message method instead:

public static void ReportError(Exception exceptionRaised, string reference, 
    string customMessage, bool sendEmail)
{
    string errorMessage = BuildMessage(exceptionRaised, customMessage);
    ReportError(errorMessage, reference, sendEmail);
}

Disclaimer: Not entirely sure if this will work. It depends how you build up the error message.

EDIT:

Or you could add a third overload:

private static void ReportError(string completeException, bool sendEmail)
{
     // Do what needs to be done.
}

And then your methods could just both build up the exception message and pass that string and sendEmail boolean to the third overload.

Answer 2

Can you just make the overload with fewer parameters call the one with more, passing in null or "" as the custom message?

public static void ReportError(string errorMessage,
                               string reference,
                               bool sendEmail)
{
    ReportError(errorMessage, reference, null, sendEmail);
}

Note that if you're using C# 4, you could do this without overloading, using an optional parameter.

Answer 3

void report(Exception e)
{
  //convert exception to plain text
  string text = e.ToString();
  //re-use the other overload
  report(text);
}

void report(string text)
{
  ...etc...
}

A more complicated version:

delegate string GetString();

public void report(Exception e)
{
  GetString getString = delegate() { return e.ToString(); }
  report(getText);
}

public void report(string text)
{
  GetString getString = delegate() { return text; }
  report(getText);
}

void report(GetString getString)
{
  ...etc...
  string text = getString();
  ...etc...
}

Answer 4

One simple solution would be to extract the duplicate code into it's own method, like this:

public static void ReportError(Exception exceptionRaised, string reference, string customMessage, bool sendEmail)
{
    // build up message from exception, reference & custom message using string builder
    ProcessError(Message, SendEmail)
}

public static void ReportError(string errorMessage, string reference, bool sendEmail)
{
    // build up message from errorMessage & reference string builder
    ProcessError(Message, SendEmail)
}

private static void ProcessError(string message, bool sendEmail)
{
    // get filename
    // check if logfile exists, blah, blah
    // save message
    // email error (if set)
}

This is certainly not the most elegant way to do it, but it is a start ;-)

Answer 5

When overloading methods, make sure that the first parameters always are the same. It's a bit confusing otherwise. In your case, make the exception the last parameter.

public static void ReportError(string customMessage, string reference, bool sendEmail, Exception exceptionRaised) 

As for code duplication. Make both methods call a third protected method which does the actual handling. The exception method can format the exception and add it to the message parameter before calling the third method.

Answer 6

The other answers are good but I thought I would add something to avoid that I have come across that tends to be a maintenance pain in the ...

public void report(string text)
{
   ...
   report(text,defaultvalue);
}

public void report(string text, string email)
{
   ...
   report(text,email,null);
}

public void report(string text, string email, List<string> errors)
{
   ...
}
...
etc.

This tends to be very anoying when you have to refactor all these methods just cause you want to change a parameter in one of the methods.