C# either return false or do nothing

问题:

I wondering if there's a way to not have to repeat the same if construction but rather call a StatusCheck(). It can't return true when it succeeds. Anyone knows a better title for this question?

bool Enable()
{
    if (!GetStatus(ref status)) { Trace.WriteLine("Error"); return false; }
    // do stuff

    if (!GetStatus(ref status)) { Trace.WriteLine("Error"); return false; }
    // do more stuff

    if (!GetStatus(ref status)) { Trace.WriteLine("Error"); return false; }
    // do even more stuff

    // 6 more times the above

    return true;
}

回答1:

You can create a CheckStatus() method that throws an exception if the status is not valid, then handle that exception in your Enable() method:

public void CheckStatus(int status)
{
    if (!IsValidStatus(status)) {
        throw new InvalidStatusException(status);
    }
}

public bool Enable()
{
    try {
        CheckStatus(status);
        // do stuff

        CheckStatus(status);
        // do more stuff

        CheckStatus(status);
        // do even more stuff

        // 6 more times the above

        return true;

    } catch (InvalidStatusException) {
        Trace.WriteLine("Error");
        return false;
    }
}

回答2:

I would have the code represented by the "Do stuff" comments refactored out into methods. You call those methods and either catch exceptions or check the return value of those methods (which may be the current status), rather than repeatedly calling GetStatus().

Also, I don't understand why a method called GetStatus() would have a ref parameter that would seem to be updated by the method with the current status? If you have to have the GetStatus() method I'd have GetStatus() take no arguments, and actually return the current status instead.

I.e.

Status status = GetStatus();

If you chose to allow those methods to throw exceptions then be careful that you don't start applying real logic when those exceptions are thrown - exceptions are not the right way to control program flow.

回答3:

Depending on how serious you want to be you could build some kind of action-queue for it breaking out the "do-stuff" in methods.

protected delegate void DoStuffDelegate(ref {TYPE} statusToCheck);

bool Enable()
{
  List<DoStuffDelegate> stuffToDo = new ...
  stuffToDo.Add(myFirstMethod);
  stuffToDo.Add(mySecondMethod);
  foreach(var myDelegate in stuffToDo)
  {
    if(!GetStatus(ref status)) { Trace.WriteLine("Error"); return false; }
    myDelegate(ref status);
  }
}

Both for good and bad C# doesn't allow any other construct (like preprocessor defines or such that we´ve got in C++).

回答4:

How about this? It requires minimal changes to your existing code. yield returnis your friend; let the compiler do all the dirty work:

IEnumerable<bool> StatusChecks()
{
    // Do stuff
    yield return GetStatus( ref status );
    // Do stuff
    yield return GetStatus( ref status );
    // Do stuff
    yield return GetStatus( ref status );
}

bool Enable()
{
    foreach ( var b in StatusChecks() )
    {
        if ( !b )
        {   
            Trace.WriteLine("Error");                       
            return false;
        } 
    }
    return true;
}

or, if you don't mind a LINQ-query with side effects, simply:

bool Enable()
{
     var result = StatusChecks().All( b => b );
     if ( !result )
     {
         Trace.WriteLine("Error");                       
     }
     return result;
}

回答5:

You could use exceptions, and let the exception propagate upwards.

void GetStatusAndException(ref Status) {
    if (!GetStatus(ref status))) Throw new Exception("Status exception");
}

If you don't want exceptions, there's nothing much you can do about that, except putting everything except the return into the method:

bool GetStatusAndTrace(ref Status) {
    bool result = GetStatus(ref status))
    if (!result) Trace.WriteLine("Error");
    return result;
}

bool Enable()
{
    if (!GetStatusAndTrace(ref status))  return false; 
    // do stuff

    if (!GetStatusAndTrace(ref status))  return false; 
    // do more stuff

    if (!GetStatusAndTrace(ref status)) return false; 
    // do even more stuff

    // 6 more times the above

    return true;
}

回答6:

A possible solution is to create a "sanity" wrapper for the API you're working with:

class Wrapper
{
    public void DoStuff() // Add static modifiers, parameters and return values as necessary
    {
        API.DoStuff();
        CheckStatus();
    }

    public void DoSomeOtherStuff()
    {
        API.DoSomeOtherStuff();
        CheckStatus();
    }

    private void CheckStatus()
    {
        Status status = default(Status);
        if(!GetStatus(ref status)) throw new InvalidStatusException();
    }
}

A little bit of work, but it allows your client code to be much more readable:

bool Enable()
{
    try
    {
        Wrapper.DoStuff();
        Wrapper.DoSomeMoreStuff();
        return true;
    }
    catch(InvalidStatusException)
    {
        Trace.WriteLine("Error");
        return false;
    }
}

回答7:

Now you don't need to use exceptions and it is still decent to read. No matter what, you still have to call something. Exceptions cost about 4000-8000 clock cycles.

bool Enable()
{
    if (GetStatus(ref status)) 
    { 
        // do stuff
        if (GetStatus(ref status)) 
        { 
            // do stuff
            if (GetStatus(ref status)) 
            { 
                // do stuff
                return true;
            }
        }
    }

    Trace.WriteLine("Error"); 
    return false;
}

回答8:

With LINQ, you can do something like:

delegate bool StatusFunc( ref int status );

bool Enable()
{
    var funcs = new StatusFunc[] 
    {
        GetStatusA,
        GetStatusB,
        GetStatusC
    };
    return funcs.All( f => f( ref status ) );
}

Note it's a bit of a hack, because GetStatus functions have side effects (modifying status), which is a no-no in LINQ-queries.

回答9:

Wrap the code in a try-catch block, and rewrite your GetStatus() to throw an exception on error. If you want to do it by the book, implement your own exception inheriting from System.Exception. Then you can also add the "status" property to the exception, if you need that information later on (your code example does not indicate so however).

Something like this:

bool Enable()
{
    try
    {
        GetStatus(ref status);
        // do stuff

        GetStatus(ref status);
        // do more stuff

        GetStatus(ref status);
        // do even more stuff

        // 6 more times the above
        return true;
    }
    catch (Exception ex)
    {
        Trace.WriteLine("Error"); 
        return false;
    }
}

Like I said, the exception thrown and caught should really be a custom exception, but the above will work.

回答10:

Let's see if I understood you right.

You can use the Specification design pattern: http://en.wikipedia.org/wiki/Specification_pattern

Instead of using harcoded validation, you should use something like an implementation of above design pattern, since provides you a more extensible way of adding rules against object states.

For example, you'd define an specification "StatusSpecification" which "does some stuff for validating if status is the right one". The result could be boolean.

A broken rules evaluator would take that boolean result and do the needed decisions (throw exception? try to recover? end application? just notice user about the error? report error?).

Summarizing: you'd need to have some "specifications loader" and execute an object state against loaded rules for the type of object. In the end, in this case, you're going to evaluate that state in a single line (from the consumer point of view), and maybe, the boolean result could be your property flag.

回答11:

bool Enable()
{
    try {
    if (!newProc }
    // do stuff

    if (!newProc }
    // do more stuff

    if (!newProc }
    // do even more stuff

   // 6 more times the above

   return true;
   }
   catch {
       return false;
   }
}

public function newProc()
{
    if (!GetStatus(ref status)) { Trace.WriteLine("Error"); return false; }

    throw new Exception
}