0

Having, say, 5 (or more) tasks (methods) that return true or false; I'm after an algorithm syntax to break the chain, if any of the consecutive methods return false, but continue inward otherwise. Currently, I've constructed a bunch of if blocks chained one inside another; however, seems ugly to me and hard to read with a bunch of arguments.

if (Method_1_Returns(String manyArgs, ...) == true) {
    if (Method_2_Returns(String manyArgs, ...) == true) {
        if (Method_3_Returns(String manyArgs, ...) == true) {
            if (Method_4_Returns(String manyArgs, ...) == true) {
                ...
            }
            else {
                MethodToShowError(someString_4);
            }
        }
        else {
            MethodToShowError(someString_3);
        }
    }
    else {
        MethodToShowError(someString_2);
    }
}
else {
    MethodToShowError(someString_1);
}

I'm planning to convert this into a switch block but, besides hating the goto directive, that doesn't seem in accordance with coding standards (or, is it?).

String temp = "1";

switch (temp) {
    case "1":
        if (Method_1_Returns(String manyArgs, ...)) goto case "2";
        MethodToShowError(someString_1);
        break;
    case "2":
        if (Method_2_Returns(String manyArgs, ...)) goto case "3";
        MethodToShowError(someString_2);
        break;
    case "3":
        if (Method_3_Returns(String manyArgs, ...)) goto case "4";
        MethodToShowError(someString_3);
        break;
    case "4":
        if (Method_4_Returns(String manyArgs, ...)) goto case "5";
        MethodToShowError(someString_4);
        break;
    ...
    default:
        break;
    }

Does any better alternative exist in C#? Is the switch block alternative acceptable?

ssd
  • 2,340
  • 5
  • 19
  • 37
  • I'm 99% sure if you chain your methods in one `if` condition (i.e. `if(Method1()||Method2()...)`) and use a reference variable to track which one failed, C# will optimize and get out of the `if` chain at the first case of a `true` if they are all chained by `||` – Mimakari Jan 31 '20 at 01:40
  • @Mimakari make that a 100% – jalsh Jan 31 '20 at 01:41
  • @jalsh vaguely remembered hearing about that trick in C# 101 – Mimakari Jan 31 '20 at 01:42
  • 1
    Related: [How to avoid if chains?](https://stackoverflow.com/questions/24430504/how-to-avoid-if-chains?page=1&tab=Votes) and [arrow anti-pattern](http://wiki.c2.com/?ArrowAntiPattern) – John Wu Jan 31 '20 at 01:44
  • 1
    @Mimakari: I am very cautious about the solution. Concatenating with `||` would spit out `true` if all methods fail but only one returns `true`; not to mention the mess that would be introduced and destroy the overall readability, which I'm after for, if concatenated with `&&`. – ssd Jan 31 '20 at 02:07
  • @Mimakari what's wrong with _[simple `&&` logic](https://stackoverflow.com/a/24430662/585968)_? –  Jan 31 '20 at 02:44
  • Possibly the chain of responsibility pattern? https://en.m.wikipedia.org/wiki/Chain-of-responsibility_pattern – 0909EM Jan 31 '20 at 02:10
  • @MickyD that's the solution I'm proposing? Or rather the inverse considering I said ||. – Mimakari Jan 31 '20 at 12:34
  • @Mimakari Just use `&&` as was already pointed out by John Wu above. This isn't one of those interview questions where they ask you to add up numbers but without using `+`. –  Feb 01 '20 at 01:07

2 Answers2

7

The simplest advice here is to fail fast:

if (!Method_1_Returns(String manyArgs, ...)) {
    MethodToShowError(someString_1);
    return;
}

if (!Method_2_Returns(String manyArgs, ...)) {
    MethodToShowError(someString_2);
    return;
}

if (!Method_3_Returns(String manyArgs, ...)) {
    MethodToShowError(someString_3);
    return;
}

if (!Method_4_Returns(String manyArgs, ...)) {
    MethodToShowError(someString_4);
    return;
}

Beyond that, it really depends on the chain of methods being called. If they all have the same signature and take the same args, then they could be put into an array and called in a loop.

Jonathon Reinhart
  • 132,704
  • 33
  • 254
  • 328
1

You need to separate concerns to smaller pieces of code:

public bool Method1(String manyArgs, ...)
{
  .. some logic ..
  if(!successful)
     MethodToShowError(someString_1)
  return successul;
}

public bool Method2(String manyArgs, ...)
{
  .. some logic ..
  if(!successful)
     MethodToShowError(someString_2)
  return successul;
}

public bool Method3(String manyArgs, ...)
{
  .. some logic ..
  if(!successful)
     MethodToShowError(someString_3)
  return successul;
}

public bool Method4(String manyArgs, ...)
{
  .. some logic ..
  if(!successful)
     MethodToShowError(someString_4)
  return successul;
}

... your frame construct ...

if (Method_1(String manyArgs, ...) &&
    Method_2(String manyArgs, ...) &&
    Method_3(String manyArgs, ...) &&
    Method_4(String manyArgs, ...))
{
    ...
}
G.Y
  • 6,042
  • 2
  • 37
  • 54
  • This of course only works if the OP has access to and is able to change the code in each method. And if your suggestion does not break any MVC requirements that might apply. – Barns Jan 31 '20 at 02:07
  • @Barns why is _"MVC"_ relevant here? –  Jan 31 '20 at 02:42
  • @MickyD I stated "if" I didn't state "because". The method named "MethodToShowError" certainly sounds like a method used to visually display an error message, directly. If the requirements of the project are to enforce the MVC design pattern and the methods like "Method4()" belong to the "Model" then you would not want the "Model" to bypass the function of the "View". Really, I know nothing about the method "MethodToShowError"...hence the warning "if". – Barns Jan 31 '20 at 03:54
  • @Barns OP has issues with the `if` structure, not the `Method_xx_Returns` methods. MVC and/or websites was not mentioned by OP (plus one has only to look at the return type to confirm). So a reader, reading the very first comment along the lines of _"This of course only works if..."_ is somewhat unnecessarily alarmed by a warning based on non-existent pre-conditions. –  Jan 31 '20 at 04:45