Fat ViewModels for editing images

How should I deal with ViewModels that are "fat"? Or should I just accept it sometimes.

Example - one of my ViewModels (editing image page, almost 300 lines of code) which has a lot of UI commands like cropping, quick image filters (like contrast, brightness), undo, redo etc.

```
public class ImageEditViewModelWinRt : Shared.ViewModel.ImageEditViewModel
{
private readonly FilterStorage _filterStorage;

public ImageEditViewModelWinRt(FilterStorage filterStorage, ViewServices viewServices)
: base(viewServices)
{
IsFadeInAnimationEnabled = true;
_filterStorage = filterStorage;
QuickFix = new QuickImageFixProperties();
InitializeCommands();
}

private void InitializeCommands()
{
AddFilterCommand = new RelayCommand(() =>
{
string addFilterKey = ViewServices.ResourceDictionary.ImageEditPageDictionaryKeys.AddFilterPageKey;
NavigateTo(addFilterKey, FilteredImage);
}, () => !IsOperationInProgress);

UndoCommand = new RelayCommand(async () => await TryRunAsyncOperation(UndoChangesExecute), () => !IsOperationInProgress);
RedoCommand = new RelayCommand(async () => await TryRunAsyncOperation(RedoChangesExecute), () => !IsOperationInProgress);
SaveChanges = new RelayCommand(async () => await TryRunAsyncOperation(SaveChangesToCurrentFileExecute), () => !IsOperationInProgress);
SaveChangesToPickedFile = new RelayCommand(async () => await TryRunAsyncOperation(SaveChangesToPickedFileExecute), () => !IsOperationInProgress);
CancelEditing = new RelayCommand(CancelEditingExecute, () => !IsOperationInProgress);
CropImageCommand = new RelayCommand(async () => await TryRunAsyncOperation(CropImageCommandExecute), () => !IsOperationInProgress);
RotateImageCommand = new RelayCommand(async (angle) => await TryRunAsyncOperation(RotateImageCommandExecute, angle));
FlipHorizontalImageCommand = new RelayCommand(async

When faced with a class you think is too large, it's best to apply the Single Responsibility Principle. So let's give it a go.

What is this class called? ImageEditViewModelWinRt

From this, I draw the assumptions that it is the WinRt version of a view model tailored to image manipulation. As such, this class should only contain image manipulation view model logic.

What does the class do? I did a quick tally and it appears to have the following functionality:

• Undo / Redo / Cancel changes

• Saving / Loading

• Cropping / Rotating

• Quick fix (whatever that is, I'm guessing an auto-rotate, but maybe it plays with colours and levels, etc?)

So it looks like this could be split into three (maybe four) ViewModels. That ought to be a great start. For communicating between the ViewModels I recommend using a messenger or dependency injection to keep coupling loose, although for things like a save/load ViewModel or an Undo/Redo ViewModel inheritance is probably a wise bet.

As for other aspects of your code (which mostly seem great to me):

bool hasImageBeenRefreshed = true;

Prefer to use var for local declarations when the right-hand side makes the type obvious. This is really helpful when you decide to change something's type later. You actually do this a lot, but not all the time.

string errorMsg = "";

Again a candidate for var, but also prefer to use string.Empty over "" because it gets your intent across better, and can't be confused with " " when skim-reading.

Lastly I really don't like underscore notation, I find it a strong detractor of readability, but that's a matter of personal preference.

StackExchange Code Review Q#59633, answer score: 2