Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have some ViewModels that have a lot of parameters like this one:

public class ProductsMainViewModel : NotificationObject, IActiveAware, ...
{
    public ProductsMainViewModel(IProductRepository productRepository, IWarehouseRepository warehouseRepository, IUnitOfMeasureRepository unitOfMeasureRepository,
            IProductCategoryRepository productCategoryRepository, IProductPriceRepository productPriceRepository, IPriceLevelRepository priceLevelRepository,
            ICodingRepository codingRepository, ICategoryRepository categoryRepository, IDialogService dialogService, Logging.ILoggerFacade logger, IRegionManager regionManager,
            IJsonSerializer jsonSerializer, ICodeService codeService, IEventAggregator eventAggregator)
    {
        // ...
    }
}

(All repositories have the same instance of IUnitOfWork) Instantiating this is done by IUnityContainer, But there are some dialogs that get called in this ViewModel:

            var viewModel = new ProductViewModel(_warehouseRepository, _unitOfMeasureRepository,
                _productCategoryRepository, _priceLevelRepository, _productPriceRepository, _codingRepository, _dialogService, _codeService, _jsonSerializer);

            var result = _dialogService.ShowDialog("ProductWindow", viewModel);
            if (result == true)
            {
                 // ...
            }

that their repositories must have the same IUnitOfWork of the current ViewModel. So I decided to pass parameters manually. I found it a good idea to "not reference any IoC container" in my ViewModel in here.

The benefit of this approach is that the ViewModel doesn't have any knowledge of the (Unity) container. Also, in terms of understanding the application design the dependencies of the ViewModel are easy to see since they are in the constructor.

Well, any ideas on how to reduce the number of parameters? Any particular design pattern?

Is it a good idea to create a 'IFooRepositoriesContext' and put related repositories in it?

share|improve this question
2  
"Is it a good idea to create a 'IFooRepositoriesContext' and put related repositories in it?" This is the route I'd go. –  Jesse C. Slicer Sep 28 '12 at 15:41

2 Answers 2

up vote 1 down vote accepted

I agree with what Jesse said in that a grouping of your repositories into one related is one approach and may be the one best suited in your case. However I'd like to offer another approach and that would be use composition in your ViewModel and have it contain lots of smaller ViewModels.

Also, in terms of understanding the application design the dependencies of the ViewModel are easy to see since they are in the constructor

I don't think that splitting the ViewModels up into smaller ones will break this statement . However I also don't necessarily agree with it either. Why should the ViewModel depend on anything?

However what I do think splitting your ViewModels up (where possible of course) is allow you to potentially re-use any Viewmodel while also making each view model specific to a particular part of the design/functional requirement.

So if I was to continue on with the IFooRepositoriesContext approach I might consider something like this:

public ProductViewModel {
     public string ProductOnlyDetail { get; set; }
     public ProductDescriptionViewModel Description { get; set; }
     public ProductPriceViewModel Price { get; set; }
}

or if you wanted to make it explict that the ProductViewModel always needs parameters then create the constructor to take the IFooRepositoriesContext:

public ProductViewModel(IFooRepositoriesContext context) {
   // and build your viewModel there
}

Your sub viewmodels themselves might look like (examples only of course):

public ProductDescriptionViewModel {

     public ProductDescriptionViewModel(IProductRepository productRepository, IWarehouseRepository warehouseRepository, IUnitOfMeasureRepository unitOfMeasureRepository,
            IProductCategoryRepository productCategoryRepository)
     {

     }

     public string Name { get; set; }
     // etc
}

public ProductPriceViewModel {
     public decimal Price { get; set; }
     public string Currency { get; set; } 
     // etc

     public ProductPriceViewModel (IProductPriceRepository productPriceRepository, IPriceLevelRepository priceLevelRepository)
     {

     }
}

Then your code might be sonething like:

var viewModel = new ProductViewModel
{
    Description = new ProductDescriptionViewModel(productRepository, warehouseRepository, unitOfMeasureRepository,
            productCategoryRepository),
    Price = new ProductPriceViewModel(productPriceRepository, priceLevelRepository)

    ProductOnlyDetail = "Hello"    
}

var result = _dialogService.ShowDialog("ProductWindow", viewModel);
if (result == true)
{
     // ...
}

But there are some dialogs that get called in this ViewModel

I assume this was a grammatical error and you don't actually call dialogs from within the viewModel itself? If so I seriously think you should reconsider your approach to using the ViewModels.

ViewModelBuilder or someother approach.

Aside from this I would consider having another object that was responsible for building up your viewmodels. Of course it depends on the design and requirements of the system but I would be asking.

Will the viewmodel always get the data from x? Is there any reason why it couldn't get it from y?

If yes then having the viewModel build itself is really limiting it to the knowledge of where and what data to obtain, even if the where of that data is abstracted out view interfaces. There are existing mapping frameworks out there already that may offer assistance if you were worried about the class explosion this might cause?

Hope this helps give a different perspective or some more food for thought :)

share|improve this answer
    
When I call _dialogService.ShowDialog("ProductWindow", viewModel); a dialog window pops up. What's wrong with that? You can mock or test it simply! –  Jalal Oct 1 '12 at 11:26
    
@Jalalx That's different to what you said. You said some dialogs get called "in" this viewModel. Not the viewModel is passed to some dialog service log and is "used". Probably just english semantics but does help to clarify your question. –  dreza Oct 1 '12 at 20:07

why don't you write an abstract factory containing all the references you need? You can then implement a factory method for your ViewModel creating an instance based on some configuration parameters. Your client code now doesn't need to know the constructor sintax but just the configuration needed for the right instance.

share|improve this answer
1  
Thank you for answer, but I choose what @Jesse said before :) –  Jalal Sep 28 '12 at 16:45

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.