1
votes

Background

I think I have the answer to this one already, but I'm trying to adhere as closely to MVVM as I can, and learning a ton a long the way.

I currently have my View, ViewModel, and Model set up. My Model is querying a database using entity framework.

My View has a bunch of controls to allow the user to set up the parameters of the query (basically building a big where clause).ViewModel stores these options set through the controls.

So my view-view model interaction seems pretty spot on, and I think it's acceptable.

The Problem

My model exposes a function which returns the results of the query as some kind of IEnumerable. The problem I'm now having is the number of "search criteria" I'm having the user set. I now have 9 parameters for the model function. I don't know if it's acceptable. At the very least, it's ugly. Very ugly. But this way, my viewModel only needs to hold an instance of the model, and then only needs to know that one function and its signature.

The Question

Should I be setting up properties in the Model and then setting those properties in the View Model? This way the single function will be much cleaner, but the view model will have to be more "aware" of what properties the model has. I know this isn't a big deal to just create some public properties, but I would like to know which is more acceptable for MVVM. Our current code base has no separation of concerns. So I'm on my own on this one.

The Code in Question

Current Model Function:

public IEnumerable<> GetResults(string id, string inputName, DateTime? fromDate,
    DateTime? toDate, bool option1, int selectCount, bool exactMatch = true, bool showFailed = false)
{
    //QUERY HERE, returns results
}

Relevant ViewModel call:

var queryResults = MyModel.GetResults(id, inputname, FromDate, ToDate, Option1, selectCount, ExactMatch, ShowFailed);
Results = queryResults.ToList();

Proposed Model:

public string Id          {get;set}
public string InputName   {get;set}
public DateTime? FromDate {get;set}
public DateTime? ToDate   {get;set}
public bool Option1       {get;set}
public int SelectCount    {get;set}
public bool ExactMatch    {get;set}
public bool ShowFailed    {get;set}

public IEnumerable<> GetResults()
{
    //Query here, return results
}

Relevant Proposed ViewModel call:

MyModel.Id = this.Id;
MyModel.InputName = this.InputName;
MyModel.FromDate = this.FromDate;
//...etc (I put the this. to clarify the view model also has those properties).
var results= MyModel.GetResults();
2
There are a couple of things I would do otherwise as you do: 1. introduce strongly typed parameter object to get rid of the current 9 parameters, 2. create a facade whose concern would be to create your model, pass the GetResults() call, 3. Map the result to viewmodel through it's own abstraction. - kayess
+1 to @kayess. Also see automapper lostechies.com/jimmybogard/2009/01/23/… - Rohit Sharma

2 Answers

4
votes

In my understanding your use case yells for implementing the Command Query Separation. About CQS details you can see my answer here. Now that we have this as a stronghold as a base to our idea, we shall check out the next refactoring steps.

Refactoring of the argument flood

Seeing your method signature, we can clearly see that you have a lot of parameters here:

public IEnumerable<T> GetResults(string id, string inputName, DateTime? fromDate,
                                 DateTime? toDate, bool option1, int selectCount, 
                                 bool exactMatch = true, bool showFailed = false)

Now if we introduce a parameter object instead of the current nine arguments, your code would become look like as:

public IEnumerable<T> GetResults(FilterObject filterObject)

Looks much better right? Now the example parameter object is just a POCO, which looks like:

public class FilterObject 
{
    public string id { get; set; }
    public string inputName { get; set; }
    ...
}

Separating the concern of getting your results

Separating the concern and getting rid of tightly coupled model and view model. A very nice and brief introduction on queries can be found here. We create an example query handler as:

public class GetResultsQueryHandler
: IQueryHandler<FilterObject, YourModel>
{
    public GetResultsQueryHandler([pass your needed dependencies here])
    {
        //set them to local variables
    }

    public YourModel Handle(FilterObject filterObject)
    {
       // Logic to call GetResults(filterObject) and return the filled model
    }
}

Now you have a nicely separated the former GetResults() call and have your model with properties "filled".

Mapping the model to view model

The last we need to do is how to map the model to a view model instance. There are a bunch of object mappers out there, a popular one is AutoMapper. In cases like yours it makes your life much easier, literally all you need to do is set up your maps and call Mapper.Map().

In the example you have shown in your question, the model and view model property names are the same, the mapping definition should be easy as:

public static void Configure()
{
    Mapper.CreateMap<YourModel, YourViewModel>();
}

Then to get the result mapped view model, the mapping operation can be done as:

var viewModel = Mapper.Map<YourModel, YourViewModel>(model);

Where the model parameter is the filled model.

A quote about the static use case method shown in my example of AutoMapper:

The 4.2.0 release of AutoMapper marked the entire static configuration and mapping API as obsolete

This means using AutoMapper >= v4.2 creating the configuration have been changed to like:

var config = new MapperConfiguration(cfg => {
    cfg.CreateMap<YourModel, YourViewModel>();
});
var mapper = config.CreateMapper();
3
votes

I'd say, "It depends", how strongly linked are those parameters.

Can they be split into groups, then it will be better to have multiple functions. e.g. Filter by timeframe, by Severity, textual search, etc.

When they clearly belong together create a class or structure to group them and pass a single parameter.

Think About how other 'consumers' can/will use your Model in the future and make your decisions based on that/