-2

How would one refactor the following event handler to fit in the ViewModel?

private void imgSkipBack_MouseDown(object sender, MouseButtonEventArgs e)
{
    if (e.ChangedButton == MouseButton.Left)
    {
        SetNewPlayerPosition(ViewModel.MovieElapsedTime.Subtract(Settings.SkipSeconds).TotalSeconds >= 0
            ? ViewModel.MovieElapsedTime.Subtract(Settings.SkipSeconds)
            : new TimeSpan(0));
    }
}

private void SetNewPlayerPosition(TimeSpan newPosition)
{
    Player.Position = newPosition;
    AlignTimersWithSource(Player.Position);
}

private void AlignTimersWithSource(TimeSpan currentPosition)
{
    ViewModel.MovieLeftTime = Player.NaturalDuration.TimeSpan - currentPosition;
    ViewModel.MovieElapsedTime = currentPosition;
}

Where the vm is declared as follows:

public class VideoPlayerViewModel : ViewModelBase
{
    private TimeSpan _movieElapsedTime = default(TimeSpan);
    public TimeSpan MovieElapsedTime
    {
        get => _movieElapsedTime;
        set
        {
            if (value != _movieElapsedTime)
            {
                _movieElapsedTime = value;
                OnPropertyChanged();
            }
        }
    }

    private TimeSpan _movieLeftTime = default(TimeSpan);
    public TimeSpan MovieLeftTime
    {
        get => _movieLeftTime;
        set
        {
            if (value != _movieLeftTime)
            {
                _movieLeftTime = value;
                OnPropertyChanged();
            }
        }
    }
}

I have tried using Commands, but they don't seems to offer any benefit besides actually following the MVVM pattern, this version actually added only extra code in the xaml to allow me to pass the EventArgs as CommandParameter.

Deadzone
  • 793
  • 1
  • 11
  • 33
  • @MickyD I'm sorry if that's the way it sounds, I'm just looking for a way to refactor this code. I decided to let people know what I've tried to solve the problem. – Deadzone Nov 01 '17 at 00:43
  • At best, your question is too broad. But, typically you'd use `ICommand` bound to a `Button.Command` property for this sort of thing. Even handling the `MouseDown` event instead of `Click` is a bit odd. How about you explain in what _objective_ way you find `ICommand` inferior, and what benefit you are seeking that it doesn't provide. – Peter Duniho Nov 01 '17 at 00:44
  • @PeterDuniho I'm not capturing `Click` because I'm not using a Button, I'm using and Image, The reason I find the ICommand version inferior is because of all the extra code I need to write in the xaml, just to be able to get the event args passed as parameter. Besides that it does seems fine. – Deadzone Nov 01 '17 at 00:46
  • @MickyD's comment was probably because your last statement seemed like an actual question rather than a rhetorical one :) – galdin Nov 01 '17 at 00:48
  • 1
    @gldraphael I can see the confusion there, I've rephrased the last sentence . – Deadzone Nov 01 '17 at 00:53
  • Looks like an antipattern. "VideoPlayerViewModel" appears to be a view model designed to hold UI logic, not business logic. UserControls should be designed for your models or your view models. You should NOT design a view model for your UserControl. Does a TextBox have a TextBoxViewModel? **No,** and there's a very good reason why. For a real life example of this anti-pattern and why it fails so hard read [this answer](https://stackoverflow.com/a/44729258/1228). MVVM != no codebehind. Put your UI logic in your codebehind and expose DependencyProperties to which your VMs bind. –  Nov 01 '17 at 16:57
  • @Will this confuses me even further. Can you please elaborate. – Deadzone Nov 01 '17 at 17:01
  • 1
    Looks like you're trying to MVVM by taking UI-specific logic that belongs in the codebehind and cram it into a view model that has absolutely no use other than holding that codebehind. That's an antipattern, and can end up screwing you in the end. –  Nov 01 '17 at 17:04
  • @Will is it a view logic because I'm setting values, to some controls? And if that method is good to stay in the code behind of the view, how are bigger projects managing to keep their code behinds empty? Can you take a look at [mm8](https://stackoverflow.com/a/47053950/5687778)'s answer and give your opinion please? – Deadzone Nov 01 '17 at 17:16
  • 1
    You don't "keep [your] code behinds empty." MVVM != no codebehind. UI logic stays in the UI. Business logic stays in your models and view models. You don't really understand how the pattern works. If you don't take a few hours to do more research (and possibly buy a book on the subject) you are just going to cause yourself further pain. –  Nov 01 '17 at 17:23
  • @Will I see, thanks for your feedback, can you please recommend me a some website where I can read about MVVM. – Deadzone Nov 01 '17 at 17:26
  • 1
    I'd suggest searching for books on amazon. Sort the results by user reviews. –  Nov 01 '17 at 18:11

1 Answers1

1

How would one refactor the following event handler to fit in the ViewModel?

You could use an interaction trigger to invoke a command when the MouseLeftButtonDown event for the element is raised:

<Image ...>
    <i:Interaction.Triggers>
        <i:EventTrigger EventName="MouseLeftButtonDown" >
            <i:InvokeCommandAction Command="{Binding YourCommand}" />
        </i:EventTrigger>
    </i:Interaction.Triggers>
</Image>

You will need to reference System.Windows.Interactivity. Please refer to the following blog post for more informatation about this: https://blog.magnusmontin.net/2013/06/30/handling-events-in-an-mvvm-wpf-application/.

I have tried using Commands, but they don't seems to offer any benefit besides actually following the MVVM pattern ...

The benefit is that you move your application logic, i.e. what happens when the element is clicked, to the view model where it belongs.

mm8
  • 163,881
  • 10
  • 57
  • 88
  • Thank you for your answer, would please provide an example of how the command would look like? – Deadzone Nov 01 '17 at 12:10
  • Please read the blog post. You just call your SetNewPlayerPosition method in the Execute method of your command. – mm8 Nov 01 '17 at 12:50
  • How can I move my `MediaElement Player` to the ViewModel? As I'm accessing a lot of properties of this control, I'd much rather have it in the vm – Deadzone Nov 01 '17 at 13:11
  • You may implement an interface: https://stackoverflow.com/questions/10631748/mvvm-pattern-violation-mediaelement-play – mm8 Nov 01 '17 at 13:30
  • I have come across this post before, but I would really appreciate an example in my concrete situation, sorry if I'm asking for too much. – Deadzone Nov 01 '17 at 16:20
  • Instead of setting Player.Position directly, you set the Position property of an interface that the view implements. – mm8 Nov 01 '17 at 16:21