views:

289

answers:

11

Hi,

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.

Thanks in advance!

Jan

+9  A: 

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)
{
}
Ian Nelson
A: 

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&gt;
        // and
        //    <http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dncscol/html/csharp07192001.asp&gt;
        //

        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.

TK
A: 

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...

poulejapon
+1  A: 

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.

Auron
+3  A: 

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());
Mark Cidade
+7  A: 

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.

Jon Limjap
A: 

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

pauldoo
A: 

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

Mark Cidade
A: 

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.

Kevin
A: 

@All

My example pointed the answers towards error- and exceptionhandling. (And the answers made me understand that exceptions in our particular case definitely are better than returning a Status object.)

But I'm still hoping to hear more about a general language feature that allows "to write a C# class on which you have to call a certain function"

@Kevin

I prefer "We've make it impossible for You to forget..." above "We assume You don't forget to..." (see also smart pointers) Neither do I think it restricts possibilities: just calling IsOK() is enough.

@marxidad

Nice solutions, giving me an good excuse to read up on Linq, Expressions, etc. (we're not yet on .NET 3.5) But I would prefer a solution within the Status class.

@Ian

The TryMyFunction returns "great success?" via the bool and Status? Isn't that doubling information?

Thanks again for all the answers!

Jan

(How is an "answer" like this ever going to get votes? I'm still missing some concepts in Stack Overflow.)

jan
+2  A: 

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.

Weeble
I'm a bit confused why I got marked down... Did I do something wrong?
Weeble
You posted a good solution to a wrong problem. The asker should not do what he actually tries to do here.
skolima
That would be a good reason to mark down the *question*, not the answer, and leave a comment as to why it's a bad idea.
Quinn Taylor