tags:

views:

75

answers:

5

Trying to avoid Tell-don't-ask, I want to combine a bool property that I was asking before calling a method, into a new method returning bool.

I try to follow the pattern, that if a method can't perform the action implied by it's name, it will throw an exception. For example if SendMail can't send the mail, it will throw an exception.

I want this particular method to return a bool to indicate the success. And am considering if I should change the name to something like TrySendMail, perhaps looking at the method signature with the bool return type should be enough?

+4  A: 

Naming the method TrySendMail seems like a not too bad approach. However, being consistent with your overall naming sceme is the most important thing.

0xA3
Very good point about being consistent
Karsten
+5  A: 

The whole TryWhatever naming pattern seems to be a fairly recent thing coming from Microsoft, but the behavior (try to do something, throw if it fails, return a meaningful strongly-typed value if it doesn't) has been around for a long time.

In theory, TryWhatever should be used if the method takes a ref parameter that receives the result, and returns a bool. If the method fails, it returns false. If it succeeds, the result is stored in the ref parameter (always the last parameter), and returns true.

You can use the DateTime.TryParse method as an example. If the method doesn't match that pattern, it's not a candidate for this naming convention.

For me, when using this convention, consistency is key. Don't surprise developers. Some of us are very scary people!

Mike Hofer
I think this is a little different "pattern": the original method doesn't seem to return anything. It's an Action<T>, more than a Func<T>
Amittai Shapira
@Ammitai: I think Mike is referring to the `TryParse` pattern, where you pass a parameter by reference.
Groo
A: 

I remember reading a piece in Code Complete 2 (although I can't remember where abouts exactly) about naming conventions, aside from trying to keep them generic so they can be used for multiple applications they should all have some consistency. In this case calling it "SendMail" keeps it straight to the point and allows you to easily reuse it without the user having to worry about the implementation details, it's obvious what it does.

Setting the return type as a boolean should be a sure sign that the method works on a Success/Fail operation, making the "Try" part redundant.

private bool SendMail()
   {
       try
       {
         //Send mail
         return true;
       } 
       catch()
       {
          //Handle exception
       }
       return false;
    }

If you were to add the XML header for the method you can even add an explanation as to what the method does when you try to call it through IntelliSense.

Jamie Keeling
A: 

Be careful with the exceptions. For instance, if the address passed into the SendMail is badly formed, I don't think it appropriate to throw an exception. Exceptions are for exceptional behaviour, and to my mind not sending a mail because the address is badly formed is expected behaviour, not exceptional. I would definitely return a false (and maybe a string reason) if the mail isn't sent rather than throwing an exception. Throwing exceptions is not just slow, but means that the stack gets unwound so has to worry about leaving everything in a good state.

Aidan
+1  A: 

The TrySomething pattern is, as you know, used in several .NET BCL methods (various TryParse methods), and people are accustomed to it, so it shouldn't surprise anyone.

If your method's signature is simply bool TrySendMail(Mail mail), then it should be pretty obvious what's going on. And I would prefer to see that signature over something like:

bool WasMailSentSuccessfully(Mail mail);

because it is not so clear from the latter method that the method is actually sending the mail. So you are making the right naming choice if you go with the Try prefix, as far as I am concerned.

On the other hand, when I see the Try prefix, I usually expect to see the out keyword inside the argument list, which would follow these conventions:

bool TrySendMail(Mail mail, out string error);

And be used as:

string error = null;
if (!TrySendMail(mail, out error))
   Console.WriteLine(error);

This convention, although pretty common, is actually rather ugly from an OOP point of view. It is basically a method which returns two values, and instead of passing one of the arguments by reference, a proper OOP way would be to wrap all of the return values in a new class.

So I would prefer something like:

SendResult SendMail(Mail mail);

and then you could use it like this:

SendResult sendResult = SendMail(mail);
if (!sendResult.Success)
    Console.WriteLine(sendResult.Value);

where SendResult could be something like:

public class SendResult : ActionResult<string>
{ ... }

public class ActionResult<T>
{
    public bool Success { get; private set; }
    public T Value { get; private set; }
    public ActionResult<T>(bool success, T value)
    {
        Success = success;
        Value = value;
    }
}

The benefit is that later adding a bunch of additional return values does not change your method's signature.

Groo