0

I have a layered application that send commands to the business layer (actually, the application is based on ncqrs framework, but I don't think it's important here).

A command looks like this :

public class RegisterUserCommand : CommandBase
{
    public string UserName { get; set; }
    public string Email{ get; set; }
    public DateTime RegistrationDate { get; set; }
    public string ApiKey {get; set;} // edit
}

There is no logic in this class, only data.

I want to have the users type their user name, email and I want the system to use the current date to build the command.

What is best between :

  1. create a strongly typed view based on the RegisterUserCommand, then inject the date and the APi Key just before sending it to the business layer ?

  2. create a RegisterUserViewModel class, create the view with this class and create the command object based on the view input ?

I wrote the following code (for the solution n°2) :

public class RegisterController : Controller
{
    //
    // GET: /Register/

    public ActionResult Index()
    {
        return View();
    }

    [HttpPost]
    public ActionResult Index(RegisterUserViewModel registrationData)
    {

        var service = NcqrsEnvironment.Get<ICommandService>();
        service.Execute(
            new RegisterUserCommand
            {
                RegistrationDate = DateTime.UtcNow,
                Email= registrationData.Email,
                UserName= registrationData.Name,
                ApiKey = "KeyFromConfigSpecificToCaller" // edit
            }
            );

        return View();
    }


    public class RegisterUserViewModel
    {
        [Required]
        [StringLength(16)]
        public string Name { get; set; }
        [Required]
        [StringLength(64)]
        public string Email{ get; set; }
    }
}

This code is working... but I wonder if I choose the correct way...

thanks for advises

[Edit] As the Datetime seems to cause misunderstanding, I added another property "ApiKey", that should also be set server side, from the web layer (not from the command layer)

[Edit 2] try the Erik suggestion and implement the 1st solution I imagined :

[HttpPost]
public ActionResult Index(RegisterUserCommand registrationCommand)
{

    var service = NcqrsEnvironment.Get<ICommandService>();
    registrationCommand.RegistrationDate = DateTime.UtcNow;
    registrationCommand.ApiKey = "KeyFromConfigSpecificToCaller";
    service.Execute(
        registrationCommand
        );

    return View();
}

... Is it acceptable ?

Steve B
  • 36,818
  • 21
  • 101
  • 174

3 Answers3

2

I think you would be better off with option #2, where you would have a separate ViewModel and a Command. While it may seem redundant (to an extent), your commands are really messages from your web server to your command handler. Those messages may not be formatted the same as your ViewModel, nor should they be. And if you're using NCQRS as is, you would then have to map your commands to your AR methods and constructors.

While it may save you a little bit of time, I think you pigeon-hole yourself in to modeling your domain after your ViewModels, and that should not be the case. Your ViewModels should be a reflection of what your user experiences and sees; your domain should be a reflection of your business rules and knowledge, and are not always reflected in your view.

It may seem like a bit more work now, but do yourself a favor and keep your commands separate from your view models. You'll thank yourself later.

I hope this helps. Good luck!

David Hoerster
  • 28,421
  • 8
  • 67
  • 102
  • that was also my conclusion. This (little) bit of extra work make a cleaner separation. I'd like to thanks Erik and Timothy who contributed a lot in my thought in the [chat yesterday](http://chat.stackoverflow.com/transcript/3373). – Steve B Sep 13 '11 at 06:35
1

I would use number 1 and use the system.componentmodel.dataannotations.metadatatype for validation.

I created an example (answer) for another SO question Here.

This allows you to keep your model in another library, validate the fields and show the fields like you would internal/private classes with DataAnnotations. I'm not a big fan of creating a completely separate class for a view that has no additional value while having to ORM the data back to another class. (If you had additional values like dropdown list values, or default values then I think it would make sense).

Instead of

[HttpPost]
public ActionResult Index(RegisterUserViewModel registrationData)
{

    var service = NcqrsEnvironment.Get<ICommandService>();
    service.Execute(
        new RegisterUserCommand
        {
            RegistrationDate = DateTime.UtcNow,
            Email= registrationData.Email,
            UserName= registrationData.Name,
            ApiKey = "KeyFromConfigSpecificToCaller" // edit
        }
        );

    return View();
}

You can have

[HttpPost]
public ActionResult Index(RegisterUserCommand registrationData)
{

    var service = NcqrsEnvironment.Get<ICommandService>();

    registrationData.ApiKey = "KeyFromConfigSpecificToCaller";

    service.Execute(registrationData);

    return View();
}
Community
  • 1
  • 1
Erik Philips
  • 53,428
  • 11
  • 128
  • 150
  • no I can't... the command is handled asynchronously. And the date is only an exemple. Imagine I want to add an "api key" in the command... I won't be able to use only a get. – Steve B Sep 12 '11 at 14:48
  • What about setting the DateTime value in the Constructor? This would happen when MVC creates the model and loads the values. – Erik Philips Sep 12 '11 at 14:49
  • not possible for two reasons : 1. the RegisterCommand is defined in a library... again, if I want to use an api key, I have to set it from the client code... not the REgisterCommand itself. 2. The class is handled asynchreously... that means it's serialized, publishing on an event bus, then deserialized (and the ctor is called again) – Steve B Sep 12 '11 at 14:53
  • I'm a big proponent of not recreating classes. In my opinion base on your needs, I would choose number 1 but use the MetadataType (if you are using MVC 2+) Updating answer. – Erik Philips Sep 12 '11 at 14:57
  • I never used such metadata. Something is still unclear about this. I have the commands defined in one project, not the web project. I understand the metadata can help me to validate the command, but what can the metadatatype do for me when setting a property ? – Steve B Sep 12 '11 at 15:05
  • Directly answering your question, it does nothing to help with setting the property. What it does is provide a way to validate a class that is not under your control in this instance, so you don't *have* to create another class. So instead of taking your RegisterUserCommand and populating it from another class AND populating it manually, you can just populate the RegisterUserCommand after MVC creates and populates it. – Erik Philips Sep 12 '11 at 15:08
1

I would recommend putting this into the constructor of the RegisterUserCommand class. That way the default behavior is always to set it to DateTime.UtcNow, and if you need to set it to something explicitly you can just add it to the object initializer. This will also help in scenarios where you're using this class in other parts of your project, and you forget to set the RegistrationDate explicitly.

public class RegisterUserCommand : CommandBase
{
    public string UserName { get; set; }
    public string Email{ get; set; }
    public DateTime RegistrationDate { get; set; }

    public RegisterUserCommand()
    {
        RegistrationDate = DateTime.UtcNow;
    }
}

And the Controller

public class RegisterController : Controller
{
    //
    // GET: /Register/

    public ActionResult Index()
    {
        return View();
    }

    [HttpPost]
    public ActionResult Index(RegisterUserViewModel registrationData)
    {

        var service = NcqrsEnvironment.Get<ICommandService>();
        service.Execute(
            new RegisterUserCommand
            {
                Email= registrationData.Email,
                OpenIdIdentifier = registrationData.OpenIdIdentifier
            }
            );

        return View();
    }


    public class RegisterUserViewModel
    {
        [Required]
        [StringLength(16)]
        public string Name { get; set; }
        [Required]
        [StringLength(64)]
        public string Email{ get; set; }
    }
}
Timothy Strimple
  • 22,920
  • 6
  • 69
  • 76
  • check my edit, I added this api key requirement as Datetime is confusing – Steve B Sep 12 '11 at 14:58
  • Not sure what the ApiKey is used for. What are the conditions that would cause it to change? – Timothy Strimple Sep 12 '11 at 15:00
  • imagine the command is sent to an external system... each caller have its own api key (most of web services provider use such system... google maps for example) – Steve B Sep 12 '11 at 15:02
  • If it is something that has to be set in the controller then it's not really a default. If you're asking whether that data should be sent from the View, then the answer would likely be no, not if the view has nothing to do with acquiring that data. – Timothy Strimple Sep 12 '11 at 15:08
  • If all you're asking is where you should set that value, then it really depends on where you get the data. If it's specific to your controller action, then yes it should be set in your action as you have it in your example. I'm still not clear on the conditions under which you would change that value. As you have it now, it's more like a constant than a variable. – Timothy Strimple Sep 12 '11 at 15:15
  • let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/3373/discussion-between-timothy-strimple-and-steve-b) – Timothy Strimple Sep 12 '11 at 15:15