Where should I put methods that are strongly connected to the View, but are massive in size, when using the MVVM pattern?

Question

Continuing on my previous question about the cluttered state of my viewmodel How can I avoid command clutter in the ViewModel?Previous question I have a new question. For a school project I am making a image editing desktop application in WPF using the MVVM pattern. Due to all the editing options (crop resize etc) there are quite a few commands,- that invoke code-heavy methods that use the GDI+ properties and methods, as well as events. ATM the viewmodel is counting 770 lines and that kind of makes me want to cry. Where should methods such as these two (oh my, please keep in mind I started programming four months ago) go?

    private void ToGrayscale()
    {
        Bitmap template = CurrentImage.LoadedImage.ToBitmap();
        var drawing = new Bitmap(template.Width, template.Height);
        var drawingsurface = Graphics.FromImage(drawing);
        var attributes = new ImageAttributes();
        attributes.SetColorMatrix(ImageFilters.GrayScaleMatrix);
        drawingsurface.DrawImage(template, new System.Drawing.Rectangle(0, 0, template.Width, template.Height),
                                   0, 0, template.Width, template.Height, GraphicsUnit.Pixel, attributes);
        drawingsurface.Dispose();
        AddSnapshot(drawing, "Desaturate");
        CurrentImage.LoadedImage = drawing.ToBitmapImage();
        UiImageContainer.Source = CurrentImage.LoadedImage;
    }



         private void OnMouseMove(object sender, MouseEventArgs args)
    {
        if (UiImageContainer.IsMouseCaptured && args.GetPosition(UiImageContainer).X > 0 &&
            args.GetPosition(UiImageContainer).Y < UiImageContainer.Source.Height && args.GetPosition(UiImageContainer).Y > 0 &&
            args.GetPosition(UiImageContainer).X < UiImageContainer.Source.Width)
        {
            if (_rubberBand == null)
            {
                _rubberBand = new System.Windows.Shapes.Rectangle();
                _rubberBand.VerticalAlignment = VerticalAlignment.Top;
                _rubberBand.HorizontalAlignment = HorizontalAlignment.Left;
                var partiallyTransparentSolidColorBrush = new SolidColorBrush(Colors.White);
                partiallyTransparentSolidColorBrush.Opacity = 0.25;
                _rubberBand.Fill = partiallyTransparentSolidColorBrush;
                _rubberBand.Stroke = new SolidColorBrush(Colors.LightGray);
                ContentGrid.Children.Add(_rubberBand);
            }
            var width = Math.Abs(_mouseLeftDownPoint.X - CurrentImagePoint.X);
            var height = Math.Abs(_mouseLeftDownPoint.Y - CurrentImagePoint.Y);
            var left = Math.Min(_mouseLeftDownPoint.X, CurrentImagePoint.X);
            var top = Math.Min(_mouseLeftDownPoint.Y, CurrentImagePoint.Y);

            _rubberBand.Width = width;
            _rubberBand.Height = height;
            var size = new Thickness(left, top, 0, 0);
            _rubberBand.Margin = size;
        }
    }

This seems like view-specific code that doesn't really belong in a view-model. I definitely agree with the others that you could factor out the functionality into other classes, but I would also suggest that those classes be referenced by your view - not your view-model. — Eben Geer, Dec 27 '11 at 06:37

score 3 · Answer 1 · answered Dec 19 '11 at 22:16

Just for example, you can do things like this

// your original ToGreyscale modified
    private void ToGrayscale()      
    {      
        Bitmap greyscaleImage = ConvertToGreyscale(CurrentImage.LoadedImage.ToBitmap());      
        AddSnapshot(greyscaleImage, "Desaturate");      
        CurrentImage.LoadedImage = greyscaleImage .ToBitmapImage();      
        UiImageContainer.Source = CurrentImage.LoadedImage;      
    }      

// put this in another class
    private Bitmap ConvertToGrayscale(Bitmap originalImage) 
    { 
        var drawing = new Bitmap(originalImage.Width, originalImage.Height); 
        var drawingsurface = Graphics.FromImage(drawing); 
        var attributes = new ImageAttributes(); 
        attributes.SetColorMatrix(ImageFilters.GrayScaleMatrix); 
        drawingsurface.DrawImage(originalImage, new System.Drawing.Rectangle(0, 0, originalImage.Width, originalImage.Height), 
                                   0, 0, template.Width, template.Height, GraphicsUnit.Pixel, attributes); 
        drawingsurface.Dispose(); 

       return drawing
                    }

An even 'sexier' way to do this would be to make ConvertToGreyscale an extension method on Bitmap.

I definitely agree. Look for code that solves common, non-application-specific problems (such as converting an image to grayscale) and try to move that code out to utility functions. Typically, utility functions are `public static` methods on a `static` class. I agree with Malcolm that the ConvertToGrayscale method sounds like an excellent candidate to be an [extension method](http://msdn.microsoft.com/en-us/library/bb383977.aspx). — Dr. Wily's Apprentice, Dec 19 '11 at 22:30

Keith Nicholas · Answer 2 · 2011-12-19T22:11:51.843

your _rubberBand should end up being a class, RubberBand maybe,

then do _rubberBand.HandleMouseMove(...);

Basically, move things into separate methods that are not coupled to your View Model, then look at moving those methods to other classes. That process will make you think a lot about how you need to structure things. You'll find when you need to move things to other classes, those classes need to interact with your View model / UI. You'll think, 'hmm, I don't want them to directly reference my UI/VM etc' then you start thinking, well, perhaps I need to come up with some interfaces that my classes could work through..... then make my VM/UI implement those interfaces.

for instance, you may find OnMouseMove, what happens depends on "Context" and then you have a "RubberBandSelection" Context or something like that. Then you come up with a generic way of having different contexts.

score 0 · Answer 3 · answered Dec 19 '11 at 22:21

Where you have complex behaviour, think about how you can break it down into smaller pieces (separate classes).

Imagine a skyscraper building. Is it a single massive thing? Or is it a collection of smaller parts, each with a distinct job to do (columns, girders, windows, etc)? And perhaps a girder is made up of beams connected with bolts. Any system can be broken down into smaller components, until each component does a single clearly defined task.

In terms of your view class, try to separate the UI from the work that it does - for example, ToGreyscale is a great example of an "image processing function" that could be contained within a class. Other image processing functions could be written into similar classes, all derived from the same base class or interface, so that they become interchangeable components.

By organising your code in this way, you can make a very complex application from many small, simple components, and it's very easy to add new components to extend the program beyond its original design.

score 0 · Answer 4 · answered Dec 19 '11 at 22:21

Create a separate class library (dll) which will be used by different service agents used by your VMs and move all that code that is not a direct responsibility of the VM over there.

To avoid tight coupling make sure you use service agents and that these service agents implement applicable interfaces instead of referencing these new objects directly so your app does not have to reference this class library.

Use object oriented design patterns and the three pillars of OO design (encapsulation, inheritance and polymorphism) to properly organize the code in your class library and achieve DRY (do not repeat yourself) state for these objects.

Continue asking more detailed questions with that username/picture, that will ensure you get lots of good answers promptly...

Where should I put methods that are strongly connected to the View, but are massive in size, when using the MVVM pattern?

4 Answers4