15

The code below will complain

try
{
    session.Save(obj);
    return true;
}
catch (Exception e)
{
    throw e;
    return false;  // this will be flagged as unreachable code
}

whereas this will not:

try
{
    session.Save(obj);
    return true;
}
catch (Exception e)
{
    return false;
    throw e;
}

I dont get it...I thought my csc101 told me that return statements should always be the last statement in a function and that it exits the function and return control to the calling code. Why does this defy my professor's logic, and why does only one of these generate a warning?

ouflak
  • 2,458
  • 10
  • 44
  • 49
sirbombay
  • 409
  • 2
  • 6
  • 17
  • 3
    "csc101 told me that return statements should always be the last statement in a function": I'm calling this out as outmoded advice from Djikstra that was in reference to more old fashioned languages that have different resource allocation/cleanup models. – spender Sep 26 '13 at 10:24
  • C#...is java different? – sirbombay Sep 26 '13 at 10:27
  • 1
    `throw` exits your function in the same way `return` does. Anything after those two statements within the same block will never be executed. In your case, you either need to `throw e` or `return false`. If you wish to do both then you need to alter your function a bit. – Michael Aquilina Sep 26 '13 at 10:30
  • Interesting discussion of "Single entry, single exit" here: http://programmers.stackexchange.com/questions/118703/where-did-the-notion-of-one-return-only-come-from – spender Sep 26 '13 at 10:30
  • I've found this [answer](http://stackoverflow.com/questions/6371564/why-does-throwing-2-exceptions-in-a-row-not-generate-an-unreachable-code-warning) but is about 2 throwing. – Alessandro D'Andria Sep 26 '13 at 10:34
  • @michael why does visual studio allow the second block and not the first? – sirbombay Sep 26 '13 at 10:36
  • @sirbombay see the end of my answer for that - neither is strictly "disallowed"; I've checked the mono compiler - it reports warnings for both – Marc Gravell Sep 26 '13 at 10:41
  • 3
    heh; btw - I think you mean "defies", not "defiles" – Marc Gravell Sep 26 '13 at 10:47
  • The reason is simple @sirbombay, it is possible for an exception to be thrown *and caught* before the return statement in the first block. In the second block, there is nothing that is there to catch the exception you are trying to throw - so it is impossible for the return statement to ever execute. – Michael Aquilina Sep 26 '13 at 10:54
  • @MichaelAquilina no, in the first block - the `return` definitely **cannot** be reached. – Marc Gravell Sep 26 '13 at 11:46
  • Why wouldn't it? If no exception is thrown, `return true` will be executed. – Michael Aquilina Sep 26 '13 at 12:01

5 Answers5

19

return will exit the method; throw will also exit the method, assuming it is not inside the try. It can only exit once!

So regardless of the order - the first of the throw / return effectively end the method.

As more general feedback, though: if the intent is to return false upon failure, all you need is:

try
{
    session.Save(obj);
    return true;
}
catch
{
    return false;
}

Personally, I would say that this is bad code - it hides the actual problem from the caller, making it very hard to debug. It tells us nothing of why it failed. I would say that the better approach is simply to let the exception bubble. In that case, there is no point returning true, because we would never return false - and there is no point catching an exception just to re-throw it. So the entire method becomes:

session.Save(obj);

(nothing else required whatsoever)

If your question is "why does only one of these generate a warning": a fair question, but the compiler isn't required to spot either of them for you. Perhaps it should spot it. I suspect that gmcs would spot this and warn about it - the compiler in mono is far more willing to point out stupidity.

Edit: as expected, [g]mcs outputs:

Program.cs(15,13): warning CS0162: Unreachable code detected

Program.cs(28,13): warning CS0162: Unreachable code detected

for the code below - so it does indeed report both uses as warnings:

class Program
{
    static void Main() { }
    static void DoSomething() { }
    bool ReturnFirst()
    {
        try
        {
            DoSomething();
            return true;
        }
        catch
        {
            return false;
            throw; // line 15
        }
    }
    bool ThrowFirst()
    {
        try
        {
            DoSomething();
            return true;
        }
        catch
        {
            throw;
            return false; // line 28
        }
    }
}
Community
  • 1
  • 1
Marc Gravell
  • 1,026,079
  • 266
  • 2,566
  • 2,900
  • ur answer is more detailed and deserves to be the correct answer.pls kindly update to include the scenario where the intention is to let the caller know wether or not save(object) succeed...and still wants to propagate the exception upwards.thanks – sirbombay Sep 26 '13 at 10:54
  • @sirbombay that makes no sense: the way they know whether it succeeded or not is simply by **whether** an exception came up. It cannot **both** return false **and** throw an exception - that simply does not make sense. – Marc Gravell Sep 26 '13 at 11:45
  • I encountered this standart compiler stupidivity today and really thought once can it be throw statement executed even after return statement? Thank you for your answer. – Tarık Özgün Güner Sep 15 '14 at 12:38
  • As tested you can use throw statement with return as " Exception ex2; try{ ... } catch(Exception ex) {ex2 = ex; return; } finally{ throw ex2;} – Tarık Özgün Güner Sep 15 '14 at 12:51
  • As improved solution here : " Exception ex2 = null; try{ ... } catch(Exception ex) {ex2 = ex; return; } finally{ if(ex2!= null){throw ex2;}}" Sorry for spamming. – Tarık Özgün Güner Sep 15 '14 at 13:09
  • @TarıkÖzgünGüner i tried your solution but it is not working – Vineet Agarwal Jan 04 '23 at 19:19
13

You are wrong: both your examples raise the Dead code compiler error because both throw and return mark the exit point of a method and no further code is allowed beyond that point.

However, whether the compiler allows it or not, the code below either the throw or the return is still dead and will never get a chance to execute.

(NOTE: this question was initially tagged as Java and my first sentence pertains to Java compiler semantics)

Marko Topolnik
  • 195,646
  • 29
  • 319
  • 436
1

Because any code after the return statement within a code block will be unreachable.

Juned Ahsan
  • 67,789
  • 12
  • 98
  • 136
0

This answer is based on C# and may or may not be applicable to Java.

In this case, you do not actually need the return statement. throw will be the last step of the function.

In this example, both return and throw will end the current function. Regardless of which way around you put them, then first will always prevent the second from being reachable.

NOTE: The exception to when a throw statement would end the function is if it was to be wrapped in a try block. In this case, the throw function would end execution of the remaining try block code, and move to the most relevant catch block - or finally block if a catch is not applicable.

Your code should look like this:

try
{
    session.Save(obj);

    return true;
}
catch(Exception e)
{
    throw e;
}

However, there is not much point in having the try/catch anyway if all you are doing is re-throwing the exception.

To specifically answer your only question:

Why does this defiles my professor's logic?

Well either your professor is wrong, or you have misunderstood them

musefan
  • 47,875
  • 21
  • 135
  • 185
0

The "return false;" in the catch block is unreachable because of the "throw e;" just before it. When the code executes in the catch block the first line is a throw which means you immediately throw the exception to the calling method and therefore any following code does not get executed. 

try
    {
        session.Save(obj);
        return true;
    }
    catch(Exception e)
    {
        throw e; //Throws exception to calling method
        return false;  //this will be flagged as unreachable code

    }

I hope this helps.

lpdavis13
  • 228
  • 2
  • 10