What is the ideal way to write a "return" in a method

Question

I do not like methods that have multiple return lines. So I created a return value with a string result - and in each condition I write the result = something ...

But when I write the try-catch mechanism, I have to set the public result of the string. Because if I return the result to try, the compiler will throw an error and say that not all codes have a return value. If I write result = string.Empty to the end of the method, resharper says that this is invalid code. So, here is an example, and here is my question:

"What is the ideal way to write" return "in a method?"

public static string PingThatAddress(string hostAddress) { try { Ping ping = new Ping(); PingReply pingreply = ping.Send(hostAddress); string result; if (pingreply != null && pingreply.Status.ToString() != "TimedOut") { result = "Address: " + pingreply.Address + "\r" + "Roundtrip Time: " + pingreply.RoundtripTime + "\r" + "TTL (Time To Live): " + pingreply.Options.Ttl + "\r" + "Buffer Size: " + pingreply.Buffer.Length + "\r"; } else { result = string.Empty; } return result; } catch (Exception pingError) { Debug.Fail(pingError.Message + " " + pingError); } //compiler error: THERE IS NO RETURN VALUE here? } 

Answer 1

Instead, you can do it like this:

 public static string PingThatAddress(string hostAddress) { string result = string.Empty; try { Ping ping = new Ping(); PingReply pingreply = ping.Send(hostAddress); if (pingreply != null && pingreply.Status.ToString() != "TimedOut") { result = "Address: " + pingreply.Address + "\r" + "Roundtrip Time: " + pingreply.RoundtripTime + "\r" + "TTL (Time To Live): " + pingreply.Options.Ttl + "\r" + "Buffer Size: " + pingreply.Buffer.Length + "\r"; } } catch (Exception pingError) { Debug.Fail(pingError.Message + " " + pingError); } return result; } 

Then just make sure that the result value is set to what makes sense in the event of an exception.

If you are trying to stick to what Resharper warns about, do it like this:

 public static string PingThatAddress(string hostAddress) { try { Ping ping = new Ping(); PingReply pingreply = ping.Send(hostAddress); string result = string.Empty; if (pingreply != null && pingreply.Status.ToString() != "TimedOut") { result = "Address: " + pingreply.Address + "\r" + "Roundtrip Time: " + pingreply.RoundtripTime + "\r" + "TTL (Time To Live): " + pingreply.Options.Ttl + "\r" + "Buffer Size: " + pingreply.Buffer.Length + "\r"; } return result; } catch (Exception pingError) { Debug.Fail(pingError.Message + " " + pingError); } return string.Empty; } 

You cannot have things in both directions: your artificial standard should not have multiple return , which probably causes you problems with Resharper.

Answer 2

Some of the suggestions, including the first part of the accepted answer, and the original question have a strong ability to return incorrect results, especially in the case of an exception.

The problem, and we have seen it many times, is that when you have one return value, minor changes in the logic of the method will inevitably lead to a path through the code that was not expected by the original author of the method and will cause the returned variable to be incorrect installed several times in the method or will not be installed at all.

Definitely when you need to collect a value to return to the caller and then complete some additional tasks in the subsequent method, but by and large this should be the exception, not the rule.

After tracking too many errors that were introduced as a result of the desire to have a single return value, our development standards now dictate that return will be used if absolutely necessary, and the reason for this should not be carefully documented in the code along with warnings for subsequent modifiers.

An additional advantage of this approach is that if the logic of the method is changed so that the new code path leads to a “hole” in the return logic, you will be automatically notified about this by the compiler. Using a single return value, the developer must visually check every possible code path to ensure that nothing was missing.

Finally, instead of having a return value outside the exception, we require that the corresponding default value be returned from the exception handler. Thus, it is very clear what will happen in the event of an exception.

So, in our environment, your code will look like this:

 public static string PingThatAddress(string hostAddress) { try { Ping ping = new Ping(); PingReply pingreply = ping.Send(hostAddress); if (pingreply != null && pingreply.Status.ToString() != "TimedOut") { return "Address: " + pingreply.Address + "\r" + "Roundtrip Time: " + pingreply.RoundtripTime + "\r" + "TTL (Time To Live): " + pingreply.Options.Ttl + "\r" + "Buffer Size: " + pingreply.Buffer.Length + "\r"; } else { return string.Empty; } } catch (Exception pingError) { Debug.Fail(pingError.Message + " " + pingError); return string.Empty; } } 

Answer 3

So, you are claiming that the if (or another stream statement) that helps you get to this exit point is not really an exit point? The only difference between this control statement and the return is that the return is actually more visible, more readable, more convenient to maintain, and less error prone. In addition, since the return statement will work at any level, you will most likely repeat the control statement x times.

The only reason you need to have a return statement at the end is the processing of resources - getting rid of the resources you stated at the beginning. This is much more common in C / C ++ (and even in C ++ it is becoming increasingly difficult to get around). In Java, you can rely on the exception mechanism and the finally operator to handle the resources as they should be (and in Java 7, the "try with resources" function).

There are other reasons for choosing the return . One of them is the ability to mark final variables: since the compiler knows that return (or throwing for that matter) is the end of the method, you can easily insert final variables. Finite variables - once you get used to them - really make it easier to read code and help you avoid errors.

  boolean someErrorCondition = true; final int setSomething; for (int i = 0; i < 8; i++) { if (i == 7) { setSomething = 11; break; } if (someErrorCondition) { return; } } 

This will not work with your approach, because the compiler will complain that the final variable will not be set (it was the whole idea, and not getting into invalid states).

Many excellent developers and language designers opted for several return statements. I would urgently advise someone to go in a different direction.

Answer 4

In case of an exception, you do not return a value.

Answer 5

Several return statements make it easy to keep track of your code. Thus, you immediately see the different return points of the method. In contrast, a single "result" field returns you to find where the field is used after changing it. This makes it difficult to track the natural flow of the method. But be that as it may, preference is usually given to style preferences. Make sure you do not mix the two approaches.