Carmax
Carmax

Reputation: 2917

How to use an optional, generic parameter on a controller action?

I am working with a BaseController that is used for a variety of entities. They may have int or string primary keys, represented by <TPk>.

E.g.:

[HttpGet]
public ActionResult Create(TPk id)
{
    return View();
}

Everything is fine until I try and use TPk as an optional parameter.

[HttpGet]
public ActionResult Create(TPk id = default(TPk))
{
    return View();
}

It seems that the 'optional' part isn't working.

So /controller/create/2 is fine, but /controller/create gives me the following error:

The parameters dictionary contains a null entry for parameter 'id' of non-nullable type 'System.Int32' for method 'System.Web.Mvc.ActionResult Create(Int32)'

The optional works fine with an int or string id. I can call /controller/create/2 AND /controller/create.

But using a generic type argument TPk, the parameterless route no longer works.


What I've Tried

I have tried making the TPk parameter nullable, but it won't compile:

The type 'TPk' must be a non-nullable value type in order to use it as parameter 'T' in the generic type or method 'Nullable'


I have tried changing the parameter name from id to altId as per this question - no joy


I have tried calling the same method, in exactly the same way, but with non-generic parameters. E.g.:

public virtual async Task<ActionResult> Create(int id = default(int))

This worked fine.


I have tried creating a simple new project to isolate this code. (Shown below). This still gives problems with the parameterless version.


Simple Code Test

Controller

public abstract class BaseController<TPk> : Controller
{
    public ActionResult Create(TPk id = default(TPk))
    {
        return View();
    }
}


public class NewsController : BaseController<int>
{

}

Entity Classes

public class BaseDataModel<TPk>
{
    public TPk Id { get; set; }
    public string Title { get; set; }
}

public class PageDataModel : BaseDataModel<string>
{
    public string Content { get; set; }
}

public class NewsDataModel : BaseDataModel<int>
{
    public DateTime Date { get; set; }
}

Upvotes: 0

Views: 1065

Answers (1)

Christian Gollhardt
Christian Gollhardt

Reputation: 17004

Asp.net conventions are heavily based on reflection. So this might explain the behavior. I have not tested if it realy does not work, but I am sure at this state you already tried to create a new project (POC) to preclude any custom code.

Maybe it can be fixed by looking deeper into the routing (method selection) and ModelBinder source code...

I would just create a different DuplicateRecord action instead.

If you do not understand your method without this comment, it is a good indication, that your current code probably smells anyway. (You are doing to much at the same thing):

// duplicates existing record if id is passed in, otherwise from scratch

Extract the shared things to another method (maybe even a service class) and have for each difference a seperate method.


That said, the idea of a generic CrudController is lovely, I tried this myself some years ago. But in trying so I have introduced all sort of generic parameters, strategy patterns, event delegates to make all possibilities possible.

  • What happens if you need a join?
  • What happens if you need a transaction?
  • How do you handle errors?
  • What happens if your crud logic needs 1, 2, 3 ... additional parameters to decide what to do?
  • Soft Delete / Hard Delete?
  • Cascade Delete / Restrict Delete?
  • What happens if you ...

I have written so much code, it was blessing to revert to the good old non generic code. And if abstracted away in a service, the ActionMethods realy do not need to get big.

public async Task<IActionResult> CreateProduct(CancellationToken ct, ProductCreateModel model)
{
    var result = await _productService.CreateAsync(model, ct);    
    //create response with some helpers... probably some ActionFilters
}

Generics can work ofcorse in a simple crud mapping where each View has exact one Entity, but it does not scale very well. So beaware and think twice about what you realy want ;)

Upvotes: 1

Related Questions