HiveBrain v1.2.0
Get Started
← Back to all entries
debugcsharpMinor

OOP methods/functions that can return a value or an exception

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
canexceptionreturnvaluemethodsthatfunctionsoop

Problem

I'm currently working through a series of bugs in an application. Our application is written in C#/ASP.NET (on the server) and HTML/CSS/JavaScript (on the client). We are using ELMAH to log any uncaught exceptions in the system so that we may identify it later and fix it, which is what I'm currently doing.

The way ELMAH works is to log any uncaught exception, so once you catch and handle the exception, it can be omitted from ELMAH.

This article closely relates to a question I asked here on Stack Overflow.

To give a little bit of insight into this article, ELMAH logs uncaught exceptions, but sometimes you have certain scenarios where the exception is thrown to the browser and handled there. ELMAH doesn't know that it's being handled elsewhere, so it still logs the exception.

In our application, we use WCF web services to send JSON data back to the client. Say for example one of those services fails and throws an exception, we send that exception back to the browser and handle it there (and as stated, ELMAH still logs it).

So I had this idea, which might only apply to a small set of use cases, but as an idea, I wanted to know what the community thought of it.

As OO programmers know, methods/functions have a return type (or void).

Example

public void SayHello()
{
  Print("Hello");
}

public string GetMyName()
{
  return "Joe Bloggs";
}


All well and good so far, but what if I wanted my method/function to return either a value, or an exception if one was to occur? I can do this in C# using the dynamic keyword.

Example

public dynamic GetMyNameOrCryLikeABaby()
{
  try
  {
      return DoSomethingWrong(); // might throw an exception, but should return a string.
  }
  catch(Exception ex)
  {
      return ex;
  }
}


Personally I don't like this approach. I don't think it is a good use of the dynamic keyword, and it involves the developer adding in additional code to check whether the return value was (in this example) a string, or an Exception.

T

Solution

-
Doing this instead of working with exceptions the usual way (throw and catch) is a big code smell. I think I would need a really good justification for this and I'm not sure slightly improved logging is that. I would try looking more into other possibilities, like filtering Exceptions based on their type, or something like that.

-
This code comes close to reinventing the Exception monad. You might want to study that for some ideas.

-
Your type should protect its invariants. If it's supposed to contain a value or an exception, but not both, you shouldn't allow the user to use the type the wrong way.

A similar principle also applies to reading: if a value is not present, Value should throw an exception, instead of silently returning the default value. The same probably applies to Exception.

-
For a simple type like this, it often makes sense to make it immutable. Also, mutable value types are evil, you should never use them without a really good reason (and I don't see that reason here).

-
You should be very careful with implicit conversions. An implicit conversion should never lead to loss of information and it should be always valid. In your case, that's not true for the case when Exceptable contains an exception, because then the implicit conversion silently returns the default value.

So, if you want to have a conversion from Exceptable to T, it should be explicit and it should throw when an exception is present.

-
You're checking Value against null. This is a bad idea for several reasons:

-
In some cases, null can be a legitimate return value that does not indicate an error. Your type won't handle that correctly.

-
If T is a non-nullable value type, then the default value is not null, so HasValue won't work correctly. If your type is meant to support only reference types, you should make that explicit by adding the constraint where T : class to it.

-
The name “exceptable” is not great. Though I'm not sure what would be a better name.

Context

StackExchange Code Review Q#28824, answer score: 7

Revisions (0)

No revisions yet.