Interrupting long running method pattern

Question

I am currently using this somewhat tedious pattern to generate error message for user running some long operation:

string _problem;

void SomeLongRunningMethod()
{
    try
    {
        _problem = "Method1 had problem";
        Method1();
        _problem = "Unexpected error during doing something in Method2";
        if(Method2())
        {
            _problem = "Method3 fails";
            Method3();
        }
        _problem = "Not possible to obtain data";
        var somedata = Method4();
    }
    catch(Exception)
    {
        MessageBox.Show("Problem with some long running method: " + _problem);
    }
}

Either of methods may throw and I want to tell the user at which step failure occurs. This is done by setting _problem before running any of them.

In some cases I can use different Exception types to catch, but that doesn't works always, e.g. both Method1 and Method2 can throw InvalidOperationException().

This repeated code looks like a pattern. Though I can't recognize it. Any ideas? How to improve readability?

I'll just assume that the example is overly simplified and that you *do* realize that you need to show (or log) the exception details as well (or you'll have a debugging nightmare when the customer calls and tells you "Method 3 fails, what should I do now?"). Apart from that: Very good question! — Heinzi, Jul 21 '16 at 11:46
@Heinzi, yes, this is simplified piece I am interested to improve. When `Method3` fails there is good *related* message , e.g. if it's a method what query database it will tell user: "Database problem" (it doesn't make sense to show user exception, e.g. `NullReferenceException`). So that the user can report that or even **try to do** something (there are database service menu to fix common problems, etc.). In addition all problems are logged, so that I can see exactly what happens and at which line. — Sinatr, Jul 21 '16 at 11:48

score 4 · Accepted Answer · answered Jul 21 '16 at 11:53

4

You could use when in the catch to differentiate between the same exception types and to check which method threw this exception:

void SomeLongRunningMethod()
{
    try
    {
        Method1();
        if (Method2())
        {
            Method3();
        }
        var somedata = Method4();
    }
    catch (InvalidOperationException invEx) when (invEx.TargetSite?.Name == nameof(Method1))
    {
        // ...
    }
    catch (InvalidOperationException invEx) when (invEx.TargetSite?.Name == nameof(Method2))
    {
        // ...
    }
    catch (Exception ex)
    {
        // ...
    }
}

answered Jul 21 '16 at 11:53

Tim Schmelter

450,073
74
686
939

This is good idea, but this doesn't distinguish the case when one of methods is called second time. An example: `ReadFile("file1.txt")` and sometime later `ReadFile("file2.txt")`. Currently I can set different value to `_problem`, e.g. `_problem=$"Reading {fileName}";`. Any idea? I understand it's my problem to not mention this criteria in the question (still learning to ask questions properly.. sigh.. and your answer covers current question perfectly). – Sinatr Jul 21 '16 at 12:05
@Sinatr: either wrap all `ReadFile` calls in a `try-catch` or let the method return a `bool`(if it's currently `void`) or add an `out bool success`-parameter to determine if the method executed successfully. Then the code remains simple and you can build your result/error-message from the result of the booleans(f.e. `bool couldReadFile1=ReadFile("file1.txt")`). – Tim Schmelter Jul 21 '16 at 12:11
I must mention that Boolean success values are a code smell, since they usually destroy more detailed error information (why *exactly* did the method fail), making debugging a lot harder. If you go that way, please return a string with an error message instead. – Heinzi Jul 21 '16 at 12:14
@Heinzi: i just didnt want to overcomplicate the idea. It would be better to return a `FileReadResponse` object which contains an optional error-message or exception and the result(f.e. a `IEnumerable` property). – Tim Schmelter Jul 21 '16 at 12:31

score 2 · Answer 2 · answered Jul 21 '16 at 11:52

2

You could get the method that caused the exception using error.TargetSite. The only thing you need to change is your catch line: catch (Exception error)

answered Jul 21 '16 at 11:52

Florian Harwoeck

395
2
11

score 1 · Answer 3 · answered Jul 21 '16 at 11:56

I'd make a sequence of the things you want to do and run through them:

var methodList = new[]{
    new{action = (Action)Method1, identifier = "Method1"},
    new{action = (Action)Method2, identifier = "Method2"},
    new{action = (Action)Method3, identifier = "Method3"},
};
string problem = null;
foreach(var info in methodList)
{
    try
    {
        info.action();
    }
    catch(InvalidOperationException)
    {
        problem = string.Format("{0} failed", info.identifier);
        break;
    }
}
if(problem != null)
{
    //notify
}

Some methods are `Func<>`, need parameters, etc. The idea of assigning called method with message is good, but not cooked yet to the extend where it's usable. — Sinatr, Jul 21 '16 at 12:00

Interrupting long running method pattern

3 Answers3