-1

In C#, I have a method that does a bunch of tests on a value. If the value fails any tests, it must skip the rest of them (some of them have side-effects).

Consider:

public void MyMethod(string myValue)
{
  // Test A
  if(myValue != myOtherValue)
    return;

  // Test B
  if(myValue != someOtherValue)
    return;

  DoImportantThings();
}

This works, because if the value fails Test A, the method exits, and if never gets to Test B. It will only DoImportantThings() if all tests pass.

But what if, when failing a test, I have to do one thing, always?

public void MyMethod(string myValue)
{
  // Test A
  if(myValue != myOtherValue)
  { 
    DoMyOneThing();
    return;
  }

  // Test B
  if(myValue != someOtherValue)
  { 
    DoMyOneThing();
    return;
  }

  DoImportantThings();
}   

This does work, but I hate the duplication. I'm paranoid that someone will add another test in the future and not include DoMyOneThing().

Here's what I would love:

[AlwaysDoBeforeExiting(DoMyOneThing())]
public void MyMethod(string myValue)

But, I know this isn't possible.

So, DoMyOneThing() has to either be (1) before each return, or (2) at the bottom of the method, in which case I can't return from the method and instead need to find a way to short-circuit Test B (and all tests following) in the event that Test A fails.

What is the clearest way to write this code? Clearly, I can brute force this, but I'm curious if there's an elegant way to write it.

Is there an accepted/conventional way to abort the rest of a method while always ensuring some code runs? Is it -- gulp -- a try...catch...finally block?

Deane
  • 8,269
  • 12
  • 58
  • 108
  • 5
    That's what `finally` is for. I don't understand the gulp. – Donnie May 17 '17 at 22:50
  • 1
    What's wrong with try/catch/finally? – RJM May 17 '17 at 22:57
  • *I'm paranoid that someone will add another test in the future and not include DoMyOneThing().* - Delegate your paranoid feeling to unit tests – Sir Rufo May 17 '17 at 23:36
  • See marked duplicates for explanations about how you can use `finally` to execute some arbitrary block of code regardless of how the section of code exits. It seems like that's exactly what you need here. If you disagree, you need to fix your question so it's clear why that doesn't work for you. – Peter Duniho May 17 '17 at 23:41
  • Note that it is not actually clear from your question whether you intend for `DoMyOneThing()` to be executed always before returning, or only in the failed cases. You should also make sure you fix your question to make that clear. – Peter Duniho May 17 '17 at 23:43

3 Answers3

1

You can use a try \ finally block for this. Everything in the try block will run as normally, but before the method returns, it will run everything in the finally block:

public void MyMethod(string myValue)
{
    try
    {
        // Test A
        if (myValue != myOtherValue)
        {
            return;
        }

        // Test B
        if (myValue != someOtherValue)
        {
            return;
        }

        DoIfTestsPass();
    }
    finally
    {
        AlwaysDoThisThing();
    }
}
Rufus L
  • 36,127
  • 5
  • 30
  • 43
  • I am ashamed to say that I never considered a `try...finally` block without the `catch`. Thank you. – Deane May 18 '17 at 13:59
  • This seems a reverse logic of what OP wanted - this will always run `DoImportantThings` even if validation fails. Instead, OP, I believe, wanted to only have `DoImportantThings` IF validation passes, but regardless of validation, always run `DoMyOneThing()`. – LB2 May 18 '17 at 16:26
  • @LB2 You're right, I misread the question. I updated the method names for clarity. – Rufus L May 18 '17 at 16:52
0

The try-finally construct serves exactly that purpose. No matter how the flow goes, be it due to return or exception, the finally will always run. As such, you can simply wrap all your value validations inside the try block and then call your DoMyOneThing() inside the finally block.

public void MyMethod(string myValue)
{
  try 
  {
    // Test A
    if(myValue != myOtherValue)
    { 
      return;
    }

    // Test B
    if(myValue != someOtherValue)
    { 
      return;
    }
  } finally {
    DoMyOneThing();
  }

  DoImportantThings();
}  
LB2
  • 4,802
  • 19
  • 35
  • **From review queue**: May I request you to please add some context around your source-code. Code-only answers are difficult to understand. It will help the asker and future readers both if you can add more information in your post. – RBT May 18 '17 at 02:57
0

While the other answers work, they are not the best approach, in my opinion. Assuming everything in your example code resembles your production, you can use what's known as "Short-circuiting".

Short Circuit MSDN: https://msdn.microsoft.com/fi-fi/library/6373h346(v=vs.110).aspx

This will work:

if(myValue != myOtherValue || myValue != someOtherValue)
{ 
    DoMyOneThing();
    return;
}

Why does this work?

If the first part of the If-Statement is true, the second part does not run.

This works with any number of booleans in the If-Statement.

Clay07g
  • 1,105
  • 7
  • 23