MVC Action with Optional Parameters -- which is better?

Question

Are there any pros/cons of using the following two alternatives in your action signature:

public ActionResult Action(int? x) // get MVC to bind null when no parameter is provided
{
    if(x.HasValue)
    {
        // do something
    }
}

OR

public ActionResult Action(int? x = null) // C# optional parameter (virtual overload)
{
    if(x.HasValue)
    {
        // do something
    }
}

Solution 1

I have never seen the second action signature in practice and can't see any usefulness of it.

The first one usually covers all the scenarios:

• If no parameter is sent (GET /somecontroller/action), the value of the x argument will be null inside the action
• If a x parameter is sent, but it is not a valid integer (GET /somecontroller/action?x=abc), the value of the x argument will be null inside the action and the modelstate will be invalid
• If a x parameter is sent and the value represents a valid integer (GET /somecontroller/action?x=123), then x will be assigned to it.

In my examples I have used GET requests with query string parameters but obviously the same applies with other HTTP verbs and if x was a route parameter.

Solution 2

You only need to specify the optional parameter value if it is going to be anything else other than null.

MVC3 will automatically set null as the value of your parameter if nothing is specified in the overload, or in the call to the Action.

However, its worth noting that if there are any non-optional parameters after this parameter in the signature, then null would have to be specified in the call.

Therefore its best to put all optional params at the end of the signature.

Solution 3

Best Asp.net MVC solution - use action method selector

Why not simplify controller action methods by removing unnecessary code branch and have this kind of code as seen here:

public ActionResult Index()
{
    // do something when there's no id
}

[RequiresRouteValues("id")]
public ActionResult Index(int id)
{
    // do something when id is present
}

This is of course possible, as long as you provide the very simple code for RequiresRouteValuesAttribute action method selector. You can find code in this blog post that does exactly this.

By my opinion this is the best possible solution to this problem, because:

1. It simplifies code by removing unnecessary branch
2. Makes code easier to maintain (due to lower complexity)
3. Extends Asp.net MVC framework as it can and should
4. Keeps parameter types as they should be without the need to make them nullable
5. etc.

Anyway. All the details about this technique is explained in great detail in linked post.

Comments

[V4Vendetta]

I think in the first case you would need the parameter while in the other case it would assume null if none are passed, So its actually the second one which you really want (just a guess).

[Zaid Masud]

@V4Vendetta no the first case does not need an explicit parameter when invoked as a web request.

[Zaid Masud]

Please expand on what you mean when you say "is null anyway"? It's true that the MVC framework will bind it to a null value when no parameter is provided, but be aware that it's MVC doing this for you. In the second alternative we are explicitly specifying an overload.

[RickAndMSFT]

good answer but note that int? x = null does nothing. null is the default value

[Zaid Masud]

Is this only applicable to the id parameter defined in the routing definition? If the parameter was called something else, I believe the RequiresRouteValues attribute would no longer be required and the solution would be similar to my second code sample, admittedly with less branching. Thank you for the idea but you might be answering a different question here.

[Robert Koritnik]

No this is applicable to any controller action parameter. It doesn't mean that provided parameter names have to be in routing definition. They may be related to query values, post fields as well... And no, it wouldn't be similar to your second example at all. This attribute is an action method selector and not an action filter... If particular action parameter is not part of request (as part of route values, query variables or post fields) this method will not be executed at all. MVC wouldn't resolve this method as a possible candidate for request processing.

[Robert Koritnik]

@zooone9243: It is of course true, that my answer doesn't directly answer the question you've asked, but it provides a much better and cleaner solution that wouldn't get you to your question in the first place.

[Doctor Jones]

Hi @Darin, I've got a request with no parameter sent, but the value of my argument is not being set as null, instead it is the object's default value. I'd prefer a null though. Any ideas on what could be causing it? I'd open a new question but it'd probably get closed as a dupe!

[Zaid Masud]

@DoctaJonez assuming you are specifying a nullable type such as int? in the signature?

[Zaid Masud]

Robert I would suggest posting this as an answer to this question.

[Doctor Jones]

@ZaidMasud yes it's a nullable int. I figured out that it was picking up the value from elsewhere. I can't remember the exact details, but it was picking up the default value of a different field (not in the model) that happened to have the same name. Sorry I can't be more specific!

[Robert Koritnik]

@ZaidMasud: That's an old long overdue question that has been sufficiently answered and covers 90% of such cases. Action method selectors are advanced topic in MVC. Action renaming is not...

[JoeBrockhaus]

This was a great solution that I've been using for some time. However, I came across a case where it doesn't work as expected. If you want your action method to be chosen when empty/null param values are sent, ie: ?item1=&item2&=item3=value then you need to modify the IsValidForRequest() method of RequiresRouteValuesAttribute. QueryString.AllKeys will return a single null 'key' for item1=&item2=. To solve this, add the following to the if(this.IncludeQueryVariables) conditional: uniques.UnionWith(controllerContext.HttpContext.Request.Quer‌​yString.GetValues(nu‌​ll));

[JoeBrockhaus]

just to add to that previous comment, you need to make sure QueryString.GetValues(null) != null before using UnionWith(..) :: just to add to that previous comment, you need to make sure GetValues doesn't return null before Unioning with the uniques collection... if (controllerContext.HttpContext.Request.QueryString.GetValues‌​(null) != null) uniques.UnionWith(controllerContext.HttpContext.Request.Quer‌​yString.GetValues(nu‌​ll));