Where do you like to put MVC view model data transformation logic?

Question

Here is the scenario, I need to load up a view model object from a several domain objects that are being returned from multiple web service service calls. The code that transforms the domain model objects into the digestible view model object is a bit of fairly complex code. The three places that I have thought about putting it are:

1. Internal methods inside of the controller to load up an instance view model.
2. Static get method or property on the view model class itself that returns a loaded instance of view model.
3. An altogether separate builder or helper class that has a Static get method, property, or overloaded constructor that returns the loaded instance of view model.

To be clear, I don't want to use AutoMapper, or tools like it. From a best practices standpoint I'd like to know where this logic should go, and why.

EDIT

So here is what I have so far, this does give me "skinny" controller logic and separation of concerns. How can I make it better though?

    // ** Controller **
    public ActionResult Default()
    {

        var viewModel = MyViewModelBuilder.BuildViewModel(MarketType.Spot);

        return View(SpotViewUrl, viewModel);
    }

    // ** Builder **
    // Lives in MVC project under ViewModelBuilders folder
    public class MyViewModelBuilder
    {
        public static ChartsModel BuildViewModel(MarketType rateMarket)
        {
            var result = new ChartsModel
            {
                RateMarket = rateMarket,
                DateRange = new DateRange()
            };
            LoadGroupedRateLists(result, rateMarket);
            LoadCoorespondingRates(result);
            return result;
        }

        private static void LoadGroupedRateLists(ChartsModel model, RateMarket rateMarket)
        {
            var rateHistSvc = new RateHistoryService(RatesPrincipal.Current.Session.Token);
            var serviceResult = (rateMarket == RateMarket.Spot)
                                                                    ? rateHistSvc.GetSpotNationalRateHistory()
                                                                    : rateHistSvc.GetContractNationalRateHistory();

            // Break lists apart by category, and re-sort and trim.
            model.Cat1Rates = CategorizeTrimAndSort("cat1", false, serviceResult);
            model.Cat2Rates = CategorizeTrimAndSort("cat2", true, serviceResult);
            model.Cat3Rates = CategorizeTrimAndSort("cat3", false, serviceResult);
            model.Cat4Rates = CategorizeTrimAndSort("cat4", true, serviceResult);
            model.Cat5Rates = CategorizeTrimAndSort("cat5", false, serviceResult);
            model.Cat6Rates = CategorizeTrimAndSort("cat6", true, serviceResult);


            // Get Date range from results.
            var sortedRateMonths = serviceResultNational
                                    .Select(rate => rate.YearMonth)
                                    .OrderBy(ym => ym.Year)
                                    .ThenBy(ym => ym.Month);

            model.DateRange.Start = sortedRateMonths.First();
            model.DateRange.End = sortedRateMonths.Last();
        }   

        ...
    }

Osseous

Have you looked at something like automapper This will allow you to build you're mapping away from the controller and also allows injection of the automapper into the controller for testability . – Entwine

As stated above, I don't want Automapper here because it will not work for this scenario, there are several domain objects being sliced up for presentation in this instance. – Osseous

Automapper most definitely could work for this scenario. You can map from multiple objects into a single object. – Kingston

That may be, but I really want to find the best pattern for custom presentation transform code, because realistically every transform/mapping problem cannot be solved by Automapper. – Osseous

I haven't run into a custom transform/mapping problem that can't be solved by AM. Not trying to convince you to use it, just saying it is highly customizable using the fluent API (.ForMember, .ConstructUsing, etc). – Kingston

Answer 1

1 or 3, not 2. Provided that if you do #3, you do not actually let the static method do the web service calls, just let it do the mapping. Domain object(s) in, viewmodel(s) out. Prefer extension method to overloaded constructor, if the object doesn't need to track state, there's no benefit to making it non-static.

Why?

As soon as you put a logic method on the model, it ceases to be a POCO. Best practice is to treat viewmodels like boring data buckets as much as possible. Some people also try to do the mapping in a viewmodel constructor which is not a good idea once you get into any kind of complexity in the mapping.

If you only need to do the mapping in one controller, you can put it in a sub-routine. Keep in mind if you want to test the sub in isolation and keep it internal, your project will have to have InternalsVisibleTo your test project.

Update

After looking at your code, I am kind of inclined to agree with @C Sharper that this belongs neither in the controller, viewmodel, nor helper class / method. Composing this ChartsModel is very interesting code, and contains a lot of business logic. It really should be in a separate layer. Your controller should call down into that layer and delegate all of this interesting and important code to another abstraction. That abstraction should then return a domain object, as @C Sharper said. Whether you use that domain object as your viewmodel, or DTO it into a different viewmodel, is up to you. Here's how something like that might look like:

public class MyController : Controller
{
    private readonly IComposeChartData _chartDataComposer;

    public MyController(IComposeChartData chartDataComposer)
    {
        _chartDataComposer = chartDataComposer;
    }

    public ActionResult Default()
    {
        var chartComposition = new ChartCompositionSettings
        {
            MarketType = MarketType.Spot,
            Token = RatesPrincipal.Current.Session.Token,
        };
        var chartData = _chartDataComposer.ComposeChartData(chartComposition);
        var chartModel = Mapper.Map<ChartsModel>(chartData);
        return View(SpotViewUrl, chartModel);
    }
}

That is a nice lean controller body. The abstraction might then look something like this:

public class ChartDataComposer : IComposeChartData
{
    public ChartData ComposeChartData(ChartCompositionSettings settings)
    {
        // all of the interesting code goes here
    }
}

This way, your viewmodel does not need to move out into a separate layer, but you do need to create a similar object (ChartData) in that layer. The interface decouples your controller from the data it needs, and the object it returns is cohesive with your presentation data (viewmodel).

I guess I don't really see this code as business logic, but more as presentation logic. Why do you see it as business logic?

Think of your RateHistoryService class as a supplier. You receive raw materials from it, and transform those raw materials into something different, creating value in the process. This is what businesses do.

In this case, the chart visualizations are the value you are providing. Otherwise, your customers would have to sift through the raw data, trim it, categorize it, sort it, group it, etc., themselves before they could create similar charts.

I probably should have explained this earlier, but the service calls are to our own business layer already, and return domain layer business objects. It seems weird to me to have more than one business layer.

Your business layer can have its own internal layering. In this case, you can create a RateChartingService that uses the RateHistoryService to return a RateChartingResult busines object. Then map that to a ChartsModel (or like I said before, use it directly as your viewmodel).

Kingston

You suggested an extension method, what would this extension method extend, the controller (I've got multiple domain objects going in?). I was thinking just a public static method on the builder/helper that has parameters for each of the domain objects going in with a return of the view model. Also, when you say sub, I assume that to mean sub-routine ,yes? – Osseous

I guess I don't really see this code as business logic, but more as presentation logic. Why do you see it as business logic? – Osseous

I probably should have explained this earlier, but the service calls are to our own business layer already, and return domain layer business objects. It seems weird to me to have more than one business layer. – Osseous

Answer 2

I would say don't do it in your Controller. Controllers should be as "skinny" as possible. I would do it like this.

Your "Data Layer" would assign the Domain objects its properties and values. Then your subsequent Layer call it "Business Layer" will transfer your Domain Object into your ViewModel. And you will simply pass the view model to your controller without having the Controller handle any of that logic.

Separation is very important. Domain Objects should stay out of the controller, and the controllers should only care about View Models.

Inscription

I really don't want to have my viewmodels in a separate assembly unless it's really considered bad practice. I feel that this is presentation layer logic, and it should be in that layer. See my last edit which adds some more color to where I'm at currently with this. – Osseous