6
\$\begingroup\$

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<double>(async (angle) => await TryRunAsyncOperation(RotateImageCommandExecute, angle));
        FlipHorizontalImageCommand = new RelayCommand(async () => await TryRunAsyncOperation(FlipHorizontalImageCommandExecute));
        FlipVerticalImageCommand = new RelayCommand(async () => await TryRunAsyncOperation(FlipVerticalImageCommandExecute));
        ImageQuickFixCommand = new RelayCommand(async () =>
        {
            IsFadeInAnimationEnabled = false; // in case of QuickFix preview looks ugly with fade animation
            QuickFix.EnqueueCurrentFilter();
            await TryRunAsyncOperation(ImageQuickFixCommandExecute);
            IsFadeInAnimationEnabled = true;
        });
    }

    [SetCurrentOperationUserFriendlyMessageAspect(UserFriendlyMessage = "Undo operation is in progress...", IsCancellable = false)]
    private async Task UndoChangesExecute()
    {
        if (_filterStorage.Undo())
            await RefreshImageProvider();
    }

    [SetCurrentOperationUserFriendlyMessageAspect(UserFriendlyMessage = "Redo operation is in progress...", IsCancellable = false)]
    private async Task RedoChangesExecute()
    {
        if (_filterStorage.Redo())
            await RefreshImageProvider();
    }

    [SetCurrentOperationUserFriendlyMessageAspect(UserFriendlyMessage = "Cropping image is in progress...", IsCancellable = false)]
    private async Task CropImageCommandExecute()
    {
        _filterStorage.AddFilter(new ImageCroppingFilter(SelectedCroppingArea));
        await RefreshImageProvider();
    }

    [SetCurrentOperationUserFriendlyMessageAspect(UserFriendlyMessage = "Rotating image is in progress...", IsCancellable = false)]
    private async Task RotateImageCommandExecute(double angle)
    {
        _filterStorage.AddFilter(new RotateFilter(angle));
        await RefreshImageProvider();
    }

    [SetCurrentOperationUserFriendlyMessageAspect(UserFriendlyMessage = "Flipping image is in progress...", IsCancellable = false)]
    private async Task FlipHorizontalImageCommandExecute()
    {
        _filterStorage.AddFilter(new FlipImageFilter(FlipMode.Horizontal));
        await RefreshImageProvider();
    }

    [SetCurrentOperationUserFriendlyMessageAspect(UserFriendlyMessage = "Flipping image is in progress...", IsCancellable = false)]
    private async Task FlipVerticalImageCommandExecute()
    {
        _filterStorage.AddFilter(new FlipImageFilter(FlipMode.Vertical));
        await RefreshImageProvider();
    }

    [SetCurrentOperationUserFriendlyMessageAspect(UserFriendlyMessage = "Quick fix is in progress...", IsCancellable = false)]
    private async Task ImageQuickFixCommandExecute()
    {
        bool hasImageBeenRefreshed = true;

        while (QuickFix.IsQuickFixAvailable() && hasImageBeenRefreshed)
        {
            _filterStorage.AddOrModifyFilter(QuickFix.DequeueFilter());
            hasImageBeenRefreshed = await RefreshImageProvider();
        }
    }

    [SetCurrentOperationUserFriendlyMessageAspect(UserFriendlyMessage = "Saving changes in progress...")]
    private async Task SaveChangesToCurrentFileExecute()
    {
        await SaveImageChanges(FilteredImage.FolderImage.ImageFile);
    }

    [SetCurrentOperationUserFriendlyMessageAspect(UserFriendlyMessage = "Saving changes to new file...")]
    private async Task SaveChangesToPickedFileExecute()
    {
        var savePicker = new FileSavePicker() { SuggestedStartLocation = PickerLocationId.PicturesLibrary };
        savePicker.FileTypeChoices.Add(".jpg", new[] { ".jpg" });
        savePicker.FileTypeChoices.Add(".png", new[] { ".png" });

        var fileToSave = await savePicker.PickSaveFileAsync();
        if (fileToSave != null)
            await SaveImageChanges(fileToSave);
    }

    private async Task SaveImageChanges(StorageFile toFile)
    {
        string errorMsg = "";
        string errorTitle = "";

        try
        {
            IImageSaver imageSaver = FilteredImage.GetFilteredImageSaver(toFile);
            await imageSaver.SaveImage(FilteredImage.FolderImage.ImageFile, CancellationToken);
        }
        catch (FileNotFoundException)
        {
            errorMsg = "Image which you're trying to save was probably deleted in the meantime ;(\n" +
                       "You'll be transfered to the main page.";
            errorTitle = "Image couldn't be saved ;-(";
        }
        catch (InvalidImageFileException)
        {
            errorMsg = "The image you're trying to save is not valid image file. Can't do that ;(\n" +
                       "You'll be transfered to the main page.";
            errorTitle = "Image couldn't be saved ;-(";
        }
        catch (ImageDimensionsTooSmallException)
        {
            errorMsg = "Sorry you can't save that image.\nMinimal image dimension is 30 px width and 30 px height.";
            errorTitle = "Image couldn't be saved ;-(";
        }

        if (!string.IsNullOrWhiteSpace(errorTitle))
            await ShowMessageAndNavigateToMainPage(errorMsg, errorTitle);

    }

    [NavigateBackOnExitAspect]
    private void CancelEditingExecute()
    {
        _filterStorage.Clear();
    }

    [SetCurrentOperationUserFriendlyMessageAspect(UserFriendlyMessage = "Loading data is in progress.", IsCancellable = false)]
    [StoreImageFileInRecentlyUsedListAspect(ApplyToStateMachine = true)]
    public async Task LoadData(FolderImage folderImage)
    {
        string errorTitle = "";
        string errorMsg = "";

        try
        {
            await TryRunAsyncOperation(async () =>
            {
                CurrentFilters = _filterStorage.Filters;
                FilteredImage = new FilteredImageFileModel(folderImage, CurrentFilters);
                FilteredImageProvider = FilteredImage.GetFilteredImage();
                await FilteredImageProvider.Load();
            });
        }
        catch (FileNotFoundException)
        {
            errorTitle = "Can't load edit page chief ;-(";
            errorMsg =
                "The image you're trying to edit is no longer available - perhaps it was removed in the meantime. \n" +
                "You'll be transfered to the main page.";
        }
        catch (InvalidImageFileException)
        {
            errorTitle = "Can't load edit page chief ;-(";
            errorMsg = "The image you're trying to edit is not a valid image file.";
        }
        catch (ImageDimensionsTooSmallException)
        {
            errorMsg = "Sorry you can't edit this image.\nMinimal image dimension is 30 px width and 30 px height.";
            errorTitle = "Can't load edit page chief ;-(";
        }

        if (!string.IsNullOrWhiteSpace(errorTitle))
            await ShowMessageAndNavigateToMainPage(errorMsg, errorTitle);
    }

    private async Task<bool> RefreshImageProvider()
    {
        string errorMsg = "";
        string errorTitle = "";

        try
        {
            await FilteredImageProvider.InvalidateImage();
            CurrentFilters = new List<Filter>(_filterStorage.Filters);
        }
        catch (FileNotFoundException)
        {
            errorTitle = "Can't render image :(";
            errorMsg =
                "The image which you're editing is no longer available - perhaps it was removed in the meantime. \n" +
                "You'll be transfered to the main page.";
        }
        catch (InvalidImageFileException)
        {
            errorTitle = "Can't render image :(";
            errorMsg = "The image you're trying to edit is not a valid image file.";
        }
        catch (ImageDimensionsTooSmallException)
        {
            errorMsg = "Sorry you can't edit this image.\nMinimal image dimension is 30 px width and 30 px height.";
            errorTitle = "Can't render image :(";
        }

        if (!string.IsNullOrWhiteSpace(errorTitle))
        {
            await ShowMessageAndNavigateToMainPage(errorMsg, errorTitle);
            return false;
        }

        return true;
    }

    private async Task ShowMessageAndNavigateToMainPage(string message, string title, bool preservePageInNavigationStack = false)
    {
        await ViewServices.MessageService.ShowMessage(title, message);
        _filterStorage.Clear();
        NavigateToMainPageCommand.Execute(null);

        if (!preservePageInNavigationStack)
            ViewServices.NavigationService.PopRecentFrame();
    }

    public IImageSourceProvider FilteredImageProvider { get; private set; }
    public FilteredImageFileModel FilteredImage { get; private set; }
    public IEnumerable<Filter> CurrentFilters { get; private set; }
    public RelayCommand CropImageCommand { get; private set; }
    public RelayCommand<double> RotateImageCommand { get; private set; }
    public RelayCommand FlipHorizontalImageCommand { get; private set; }
    public RelayCommand FlipVerticalImageCommand { get; private set; }
    public RelayCommand ImageQuickFixCommand { get; private set; }
    public QuickImageFixProperties QuickFix { get; private set; }
    public Rect SelectedCroppingArea { get; set; }
    public Rect ImageBounds { get; set; }
    public bool IsFadeInAnimationEnabled { get; set; }
}

Most of the code is pretty straight-forward (at least for me) - except for ugly exception handling (I'd go for aspects there but with current postsharp async support it is not possible). Is there any universal solution for a "lot of commands" view models?

\$\endgroup\$
  • \$\begingroup\$ is this problem still exist? or have you got any way to solve it \$\endgroup\$ – paritosh Aug 12 '14 at 11:21
  • 3
    \$\begingroup\$ what about splitting the this view model up into a few smaller view models with different responsibilities. For example, undo / redo and image manipulation. \$\endgroup\$ – craftworkgames Sep 9 '14 at 6:25
2
\$\begingroup\$

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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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