Using return value vs out parameters

You’re asking the wrong question. In reality, both methods are bad. If you need the success message, either throw an exception on failure (but only if this is really an exceptional scenario, or return the success and indicate this in the method name. For instance, by calling the method TrySomething.

In general, your example sounds like the method is actually doing two distinct things, otherwise there would be no confusion in the first place. But this is code smell anyway: every method should have only one purpose. If that is the case, then the meaning of the return value is always clear, no need to give it a name.

I would go with

bool TryDoSomething([some arguments]);

, following the naming logic of Int.TryParse.

The first form is indeed misleading - you have no reliable way of knowing what that bool value means.

The second is better, as the parameter name (everythingIsOk) is descriptive enough.

In cases like these, however, I'd use this signature:

void DoSomething([some arguments]);

and throw an appropriate Exception if anything goes wrong.

I see two advantages to this approach:

• it doesn't mix inputs and outputs - I think it's nice that the list of arguments only contains those needed for the method to do its job
• in case something is not OK you can give extensive details about what went wrong (by throwing an appropriate type of Exception and/or by populating the exception's Message)

There's one scenario where I'd avoid this, though: if your method is called many times, the creation of the `Exception object may be expensive, resulting in a performance penalty. In most cases, however, this will not be a problem.

Update:

There's another possibility - returning a result object that has a bool property indicating success state. This is used for instance by .NET's Regex.Match(...) method (it returns a Match object that has a bool Success property).

Our method signature would be:

SomeResult DoSomething([some arguments]);

The caller could inspect the value of SomeResult.IsSuccesful to see if all went OK.

Not a good idea IMO. Too much extra work and potential gotchas for something that is easily solved with good code comments.

Using out has implications if overloading such a function. The compiler cannot differentiate between ref and out.

Cannot pass properties as out parameters.

The calling code must declare and assign an "extra" variable associated with the out parameter.

I think the main problem here is not the code itself, its that there's no xml comments. Seriously, commenting can go a long way. I'd address that first. Second, I'd look at the name of the method itself. Is it doing a check? Then CanDoSomething, IsSomething, etc would work. If the method actually does "do something" and it's returning success or failure, then the name is fine. Another alternative though might be an exception, depending on if the call should normally work or if failure is pretty common.

Regarding your suggested change with an out parameter, I couldn't do that. Its not more expressive, and it forces callers to create a variable to hold the result. In general, I think you should steer away from out / ref paramaters, they should be used pretty rarely.

I think the key question here is whether the boolean being false indicates an 'exceptional' problem or not. If it does, then forget the boolean and throw an exception.

Otherwise, you should separate out the validation from the execution:

if (canDoSomething([some arguments]) {
   doSomething([some arguments]);
} else {
   doSomethingElse([some arguments]);
}

and one might go to a factory method from there:

OperationToDoSomething operation = getAppropriateOperation([some arguments]);
operation.doIt();

How about naming the method CanDoSomething? Prefixing the method name Can, Is, Has, or Should implies a Boolean result.

Thinking in terms of what the caller will look like, I feel the first one is better.

if(DoSomething())
{
    DoSomething1();
}
else
{
    DoSomething2();
}

vs.

bool b=false;
DoSomething(b);
if (b)
{
    DoSomething1();
}
else
{
    DoSomething2();
}

or even consider if you are calling this function in return statement, it could simply be called as

return DoSomething();

Whenever I find out I need more than one output from a method, this means:

1. My method does more work than it should, so I'll try to refactor
2. The data returned deserves its own struct/class, even an private one, with 2 or more properties for the real result of the operation and the flag for success, like:

Code:

private struct ComplexOperationResult
{
   public bool IsOperationSuccessful{get;set;}
   public MyRealData RealData{get;set;}    
}

The notations like Int32.TryParse are useful, but only for some infrastructure/base operations for a framework. Previous implementations (I'm not sure for the current one) of Int32.TryParse were only wrapper around try{int32.parse}catch{..}, because this is such a common operation, and to avoid a lot of boilerplate code.

So, if it's a public framework/library method, which will be used a lot, and exposing it otherwise will lead to too much additional repetitive coding for the consumers, go for TryXYZ pattern. Otherwise, introduce proper result structure, or refactor/rethink the method.

Unless DoSomething is a trivial operation I would return a custom Result object that holds more information about what has happened. For instance:

public class Result
{
    public bool WasSuccessful { get; private set; }
    public string Message { get; private set; }
}

var result = DoSomething();

if (result.WasSuccessful)
{
   // 
}
else
{
    Console.Write(result.Message);
}

This way you can communicate back a lot more information than just a true/false value and make the properties have more meaningful values. The values I supplied are just illustrative, and obviously will change based on your requirements (as should the name of the "Result" object). It also means you can easily add extra properties/information later without breaking the method signature.

I would like to use RETURN VALUE in your case. Sometime it is really easy to use. like below ↓

.Append(MethodA()).Append(MethodB())......

I only use the OUT VALUE to save the status in each step(method) of a entire process , not for the result value.

And in C#4.0 you also can Cast the child class to parent by OUT keyword.

public delegate T Find<out T>();
Find<Cat> cat = ()=>new Cat();
Find<Animal> animal = cat;

A slightly different perspective: it's always easier to go asynchronous with the void version.

In this case is not requied to refactor but what if you need two output parameters? Example:

public bool DoSomething(object parameter, out ref string ErrorDescription)
{
try {
...
}
catch (Exception ex)
{
 ErrorDescription = ex.Message;
 return false;
}
ErrorDescription = "ok.";
return true;
}

If you would not use output parameter, you had to use any public property or create class as return type. This is easier.