Question

I have a "Status" class in C#, used like this:

Status MyFunction()
{
   if(...) // something bad
     return new Status(false, "Something went wrong")
   else
     return new Status(true, "OK");
}

You get the idea. All callers of MyFunction should check the returned Status:

Status myStatus = MyFunction();
if ( ! myStatus.IsOK() )
   // handle it, show a message,...

Lazy callers however can ignore the Status.

MyFunction(); // call function and ignore returned Status

or

{
  Status myStatus = MyFunction(); 
} // lose all references to myStatus, without calling IsOK() on it

Is it possible to make this impossible? e.g. an throw exception

In general: is it possible to write a C# class on which you have to call a certain function?

In the C++ version of the Status class, I can write a test on some private bool bIsChecked in the destructor and ring some bells when someone doesn't check this instance.

What is the equivalent option in C#? I read somewhere that "You don't want a destructor in your C# class"

Is the Dispose method of the IDisposable interface an option?

In this case there are no unmanaged resources to free. Additionally, it is not determined when the GC will dispose the object. When it eventually gets disposed, is it still possible to know where and when you ignored that specific Status instance? The "using" keyword does help, but again, it is not required for lazy callers.

Was it helpful?

Solution

I am fairly certain you can't get the effect you want as a return value from a method. C# just can't do some of the things C++ can. However, a somewhat ugly way to get a similar effect is the following:

using System;

public class Example
{
    public class Toy
    {
        private bool inCupboard = false;
        public void Play() { Console.WriteLine("Playing."); }
        public void PutAway() { inCupboard = true; }
        public bool IsInCupboard { get { return inCupboard; } }
    }

    public delegate void ToyUseCallback(Toy toy);

    public class Parent
    {
        public static void RequestToy(ToyUseCallback callback)
        {
            Toy toy = new Toy();
            callback(toy);
            if (!toy.IsInCupboard)
            {
                throw new Exception("You didn't put your toy in the cupboard!");
            }
        }
    }

    public class Child
    {
        public static void Play()
        {
            Parent.RequestToy(delegate(Toy toy)
            {
                toy.Play();
                // Oops! Forgot to put the toy away!
            });
        }
    }

    public static void Main()
    {
        Child.Play();
        Console.ReadLine();
    }
}

In the very simple example, you get an instance of Toy by calling Parent.RequestToy, and passing it a delegate. Instead of returning the toy, the method immediately calls the delegate with the toy, which must call PutAway before it returns, or the RequestToy method will throw an exception. I make no claims as to the wisdom of using this technique -- indeed in all "something went wrong" examples an exception is almost certainly a better bet -- but I think it comes about as close as you can get to your original request.

OTHER TIPS

I know this doesn't answer your question directly, but if "something went wrong" within your function (unexpected circumstances) I think you should be throwing an exception rather than using status return codes.

Then leave it up to the caller to catch and handle this exception if it can, or allow it to propogate if the caller is unable to handle the situation.

The exception thrown could be of a custom type if this is appropriate.

For expected alternative results, I agree with @Jon Limjap's suggestion. I'm fond of a bool return type and prefixing the method name with "Try", a la:

bool TryMyFunction(out Status status)
{
}

If you really want to require the user to retrieve the result of MyFunction, you might want to void it instead and use an out or ref variable, e.g.,

void MyFunction(out Status status)
{
}

It might look ugly but at least it ensures that a variable is passed into the function that will pick up the result you need it to pick up.

@Ian,

The problem with exceptions is that if it's something that happens a little too often, you might be spending too much system resources for the exception. An exception really should be used for exceptional errors, not totally expected messages.

Even System.Net.WebRequest throws an exception when the returned HTTP status code is an error code. The typical way to handle it is to wrap a try/catch around it. You can still ignore the status code in the catch block.

You could, however, have a parameter of Action< Status> so that the caller is forced to pass a callback function that accepts a status and then checking to see if they called it.

void MyFunction(Action<Status> callback)
 { bool errorHappened = false;

   if (somethingBadHappend) errorHappened = true;

   Status status = (errorHappend)
                     ? new Status(false, "Something went wrong")
                     : new Status(true, "OK");
   callback(status)

   if (!status.isOkWasCalled) 
     throw new Exception("Please call IsOK() on Status"). 
 }

MyFunction(status => if (!status.IsOK()) onerror());

If you're worried about them calling IsOK() without doing anything, use Expression< Func< Status,bool>> instead and then you can analyse the lambda to see what they do with the status:

void MyFunction(Expression<Func<Status,bool>> callback)
 { if (!visitCallbackExpressionTreeAndCheckForIsOKHandlingPattern(callback))
     throw new Exception
                ("Please handle any error statuses in your callback");


   bool errorHappened = false;

   if (somethingBadHappend) errorHappened = true;

   Status status = (errorHappend)
                     ? new Status(false, "Something went wrong")
                     : new Status(true, "OK");

   callback.Compile()(status);
 }

MyFunction(status => status.IsOK() ? true : onerror());

Or forego the status class altogether and make them pass in one delegate for success and another one for an error:

void MyFunction(Action success, Action error)
 { if (somethingBadHappened) error(); else success();
 }

MyFunction(()=>;,()=>handleError()); 

Using Status as a return value remembers me of the "old days" of C programming, when you returned an integer below 0 if something didn't work.

Wouldn't it be better if you throw an exception when (as you put it) something went wrong? If some "lazy code" doesn't catch your exception, you'll know for sure.

Instead of forcing someone to check the status, I think you should assume the programmer is aware of this risks of not doing so and has a reason for taking that course of action. You don't know how the function is going to be used in the future and placing a limitation like that only restricts the possibilities.

That would sure be nice to have the compiler check that rather than through an expression. :/ Don't see any way to do that though...

You can throw an exception by:

throw MyException;


[global::System.Serializable]
        public class MyException : Exception
        {
        //
        // For guidelines regarding the creation of new exception types, see
        //    http://msdn.microsoft.com/library/default.asp?url=/library/en-us/cpgenref/html/cpconerrorraisinghandlingguidelines.asp
        // and
        //    http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dncscol/html/csharp07192001.asp
        //

        public MyException () { }
        public MyException ( string message ) : base( message ) { }
        public MyException ( string message, Exception inner ) : base( message, inner ) { }
        protected MyException (
          System.Runtime.Serialization.SerializationInfo info,
          System.Runtime.Serialization.StreamingContext context )
            : base( info, context ) { }
    }

The above exception is fully customizable to your requirements.

One thing I would say is this, I would leave it to the caller to check the return code, it is their responsability you just provide the means and interface. Also, It is a lot more efficient to use return codes and check the status with an if statement rather than trhowing exceptions. If it really is an Exceptional circumstance, then by all means throw away... but say if you failed to open a device, then it might be more prudent to stick with the return code.

@Paul you could do it at compile time with Extensible C#.

GCC has a warn_unused_result attribute which is ideal for this sort of thing. Perhaps the Microsoft compilers have something similar.

One pattern which may sometimes be helpful if the object to which code issues requests will only be used by a single thread(*) is to have the object keep an error state, and say that if an operation fails the object will be unusable until the error state is reset (future requests should fail immediately, preferably by throwing an immediate exception which includes information about both the previous failure and the new request). In cases where calling code happens to anticipate a problem, this may allow the calling code to handle the problem more cleanly than if an exception were thrown; problems which are not ignored by the calling code will generally end up triggering an exception pretty soon after they occur.

(*) If a resource will be accessed by multiple threads, create a wrapper object for each thread, and have each thread's requests go through its own wrapper.

This pattern is usable even in contexts where exceptions aren't, and may sometimes be very practical in such cases. In general, however, some variation of the try/do pattern is usually better. Have methods throw exception on failure unless the caller explicitly indicates (by using a TryXX method) that failures are expected. If callers say failures are expected but don't handle them, that's their problem. One could combine the try/do with a second layer of protection using the scheme above, but I'm not sure whether it would be worth the cost.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top