Правильно ли я реализовал MVVM и нужен ли он тут вообще

Question

Суть приложения, брать с сайта название фильма, закачивать картинки по названию, и устанавливать их на задний фон с названием фильма.

У меня проблема в том, что я не понимаю как правильно сделать, обновление view должно же происходить во viewmodel классе?!.

Но у меня стоит таймер в 5 секунд, который каждые 5 секунд обновляет картинку и название(берет это все из List)

И получается так, что 2 переменные CurrentFilmName,CurrentFilmPicture находятся в классе viewmodel. И чтобы установить в них значения, мне нужно сделать их статичными, либо передать класс в конструктор и с ним уже работать.

Как мне сделать правильно? Перекинуть метод таймера в класс viewmodel? Передать viewmodel в класс таймера? Или класс таймера, реализовать интерфейс INotifyPropertyChanged и из него, обновлять данные.

Main

public partial class MainWindow : Window
    {
        public MainWindow()
        {
            InitializeComponent();
            VM viewmodel = new VM();
            DataContext = viewmodel;
            new TimerMain(viewmodel).StartScrollTimer();
        } 
    }

ViewModel

class VM : INotifyPropertyChanged
    {
        public event PropertyChangedEventHandler PropertyChanged;
        private void OnPropertyChanged([CallerMemberName] string prop = "")
        {
            PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(prop));
        }
    private string _filmname;
    public string CurrentFilmName
    {
        get => _filmname;
        set
        {
            _filmname = value;
            OnPropertyChanged();
        }
    }

    private string _picturepath;
    public string CurrentPicturePath
    {
        get => _picturepath;
        set
        {
            _picturepath = value;
            OnPropertyChanged();
        }
    }
}

Model(Таймер)

 class TimerMain
    {
        private Images images = new Images();
        private List<Classes.Image> listimages = new List<Classes.Image>();
        private int numbertick = 0;
        private VM viewmodel;
    public TimerMain(VM viewmodel)
    {
        this.viewmodel = viewmodel;
    }

    public void StartScrollTimer()
    {
        images.TakeImage += (Classes.Image img) =&gt; listimages.Add(img);
        // Подписываемся на событие, когда скачивается картинка, и добавляем в массив Объектов Image(В нем хранится название, и ссылка на файл картинки)
        // Запускаем парсинг фильмов, он парсит с сайта название фильма и ищет ему картинку, при успешном нахождении, скачивает, и уведомляет о новом, созданном Image
        _ = Task.Run(async () =&gt; await images.ParsFilmsAndDownload());


        DispatcherTimer timer = new DispatcherTimer();
        timer.Tick += new EventHandler(async (object obj, EventArgs e) =&gt;
        {

            while (listimages.Count &lt; numbertick + 1)
                await Task.Delay(25);


            try
            {
                viewmodel.CurrentFilmName = listimages[numbertick].FilmName;
                viewmodel.CurrentPicturePath = listimages[numbertick].FilmPicture;
            }
            catch (Exception)
            {
                viewmodel.CurrentFilmName = listimages[numbertick].FilmName;
                viewmodel.CurrentPicturePath = listimages[numbertick = numbertick == 4 ? 0 : numbertick + 1].FilmPicture;
            }

            numbertick = numbertick == 4 ? 0 : numbertick + 1;
        });

        timer.Interval = new TimeSpan(0, 0, 5);
        timer.Start();
    }
}

Сколько статей и видео не пересматривал, до конца не пойму, как правильнее, решил у вас спросить. Пока мне кажется правильной идея, реализовать TimerMain : INotifyPropertyChanged там сделать переменные, и обновить от туда(но тогда не соответствую паттерну, viewmodel должен же обновлять view).

---

Изменения:

VMFilms

 class VMFilms : INPC
    {
        private string filmName;
        public string CurrentFilmName
        {
            get => filmName;
            set
            {
                filmName = value;
                OnPropertyChanged();
            }
        }
    private string picturePath;
    public string CurrentPicturePath
    {
        get => picturePath;
        set
        {
            picturePath = value;
            OnPropertyChanged();
        }
    }
}

INPC

class INPC : INotifyPropertyChanged
    {
        public event PropertyChangedEventHandler PropertyChanged;
        public void OnPropertyChanged([CallerMemberName] string prop = "")
        {
            PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(prop));
        }
    }

Answer 1

Давайте я вам напишу небольшой пример, который покажет как реализовать MVVM подход для чтения чего либо с сервера раз в N сек.

Для начала поймем какие вы допустили ошибки:

1. Ваше окно (View слой) строго зависим от других слоев (ViewModel и Model), также оно у вас отвечает за создание других слоев, что с точки зрения MVVM не совсем правильно, ведь MVVM, это подход проектирования, где слои слабо связаны друг с другом. То есть этого кода у вас вовсе не должно быть:

    public MainWindow()
    {
        InitializeComponent();
        VM viewmodel = new VM();
        DataContext = viewmodel;
        new TimerMain(viewmodel).StartScrollTimer();
    } 
   

2. У вас Model тесно завязана на ViewModel, это неправильно и опять является нарушением MVVM, ведь Model, это отдельный слой, который делает свою задачу и отдает нужные данные наружу по требованию ViewModel, но не на оборот. Этого кода у вас быть не должно:

   new TimerMain(viewmodel).StartScrollTimer();
   

3. У вас все смешено в одну "кашу". Есть такая вещь, зовется SOLID, там правило SRP - единственная ответственность. Вот задайте себе вопрос, а ответственен ли класс VM за фильм? Он должен вообще иметь в себе методы или свойства по обработке фильма? Я думаю, нет. Так почему это не вынести в отдельный класс, который будет в себе иметь все, что нам надо?

4. У вас есть некоторые проблемы с наименованием. В C# принято именовать на понятном, английском языке все, пытаясь коротко отразить суть того или иного объекта. Ваш класс VM - это что? Также не забываем, что в C# используется CamelCase, где каждая начальная буква слова должна быть заглавной, а если это приватное значение, то первая буква делается в нижнем регистре, остальные также с заглавной. То есть filmname -> filmName, listimages -> listImages и т.д..

5. Вы не совсем, верно, работаете с асинхронностью. Зачем вам _ = Task.Run(async () => await images.ParsFilmsAndDownload());, если можно (и даже нужно) сделать public async Task StartScrollTimer()? Также если это ваш метод, то почему он не возвращает данные? По названию он должен спарсить и скачать нам фильм, то есть "парсинг" подразумевает то, что мы должны получить желаемы данные. Ну и заметьте, опять проблема с именованием, почему вдруг images (картинки) имеет метод "скачать фильм"?

---

Так, давайте теперь напишем простенький MVVM проект, который будет брать из интернета случайную картинку и выводить ее на экран раз в N сек.

1. Напишем Model.
   Я взял первый попавшийся сайт в интернете по запросу random image api, source.unsplash.com. Нам надо для него написать модель, которая будет иметь все необходимые методы для работы с сайтом.

    public class UnsplashModel
    {
        //https://source.unsplash.com/1600x900/?nature,water
    // Читаем документацию. Клиент задается один раз на все приложение!
    // Если он должен использоваться где либо еще, то стоит создать его в другом месте и передавать как ссылку.
    private static readonly HttpClient httpClient
        = new() { BaseAddress = new("https://source.unsplash.com/") };
   
    public event Action<Uri> OnNewImage;
   
    /// <summary>
    /// Найти случайное изображение по ключевым словам и размеру.
    /// </summary>
    /// <param name="size">Размер нужного изображения</param>
    /// <param name="keywowrds">Ключевые слова</param>
    /// <returns><see cref="Uri"/> найденного изображения</returns>
    public async Task<Uri> SearchImageAsync(Size size, params string[] keywowrds)
    {
        try
        {
            var responseMessage = await httpClient.GetAsync($"{size}/?{string.Join(',', keywowrds)}");
            responseMessage.EnsureSuccessStatusCode();
            var imageUri = responseMessage.RequestMessage.RequestUri;
            OnNewImage?.Invoke(imageUri);
            return imageUri;
        }
        catch (Exception)
        {
            throw;
        }
    }
   
    /// <summary>
    /// Найти случайное изображение по ключевым словам и размеру по умолчанию 1920х1080.
    /// </summary>
    /// <param name="keywowrds">Ключевые слова</param>
    /// <returns><see cref="Uri"/> найденного изображения</returns>
    public Task<Uri> SearchImageAsync(params string[] keywowrds)
        => SearchImageAsync(new(1920, 1080), keywowrds);
   
    /// <summary>
    /// Получает случайно изображение с указанными параметрами раз в <paramref name="delay"/> секунд.
    /// </summary>
    /// <param name="delay">Частота обновлений</param>
    /// <param name="size">Размер нужного изображения</param>
    /// <param name="keywowrds">Ключевые слова</param>
    /// <returns></returns>
    public async Task StartRandomAsync(TimeSpan delay, Size size, params string[] keywowrds)
    {
        while (true)
        {
            _ = await SearchImageAsync(size, keywowrds);
            await Task.Delay(delay);
        }
    }
   
   }
   

   Посмотрите внимательней, у нас простой класс, который выполняет свою логику и отдает наружу нужные данные. Он не ссылается на VM или что-либо еще. Size - это простая структура с двумя числами, не более.

    public struct Size
    {
        public Size(int width, int height)
            => (Width, Height) = (width, height);
    public int Width { get; init; }
    public int Height { get; init; }
   
    public override string ToString()
        => $"{Width}x{Height}";
   
   }
   

2. Делаем ViewModel изображения. Суть класса - содержать в себе необходимые данные изображения (может вес, ссылку, автора, размеры, да все что угодно, что относится именно к изображению), у меня он такой:

    public class ImageViewModel
    {
        public ImageViewModel(DateTime updateTime, Uri uri)
            => (UpdateTime, Uri) = (updateTime, uri);
    public DateTime UpdateTime { get; }
    public Uri Uri { get; }
   
   }
   

   INPC тут не нужен так, как мы не обновляем старое, а получаем новое.

3. Основная ViewModel, некий класс, который будет общаться с Model и составит нам свойства для привязки.

    public class MainViewModel : BindableBase
    {
        private readonly UnsplashModel unsplashModel;
    public MainViewModel()
    {
        unsplashModel = new();
        unsplashModel.OnNewImage += OnNewImage;
    }
   
    private void OnNewImage(Uri imageUri)
        => Image = new(DateTime.Now, imageUri);
   
    private ImageViewModel image;
    public ImageViewModel Image
    {
        get => image;
        set => SetProperty(ref image, value);
    }
   
    public async Task StartAsync()
        => await unsplashModel.StartRandomAsync(TimeSpan.FromSeconds(15), new(1920, 1080), "nature", "sky");
   
   }
   

   Тут уже используется INPC. Ну а логика думаю простая, понять можно без проблем. Инициализируем, подписываемся на событие, ждем, инициализируем свойство.

4. Делаем View. В нем нам надо создать нужный дизайн и указать какие свойства должны использоваться:

    <Grid>
        <Grid.RowDefinitions>
            <RowDefinition/>
            <RowDefinition Height="Auto"/>
        </Grid.RowDefinitions>
        <Image Grid.RowSpan="2"
               Source="{Binding Image.Uri}" 
               VerticalAlignment="Center"
               HorizontalAlignment="Center"
               Stretch="UniformToFill"/>
        <Border Grid.Row="1" Padding="8 10" Background="#F4F5F5F5">
            <StackPanel>
                <TextBlock Text="{Binding Image.UpdateTime, StringFormat='Обновлено: {0:dd.MM.yyyy в hh:mm}'}"/>
                <StackPanel Orientation="Horizontal">
                    <TextBlock Text="Ссылка на изображение:"/>
                    <TextBox IsReadOnly="True" Text="{Binding Image.Uri, Mode=OneWay}"
                         BorderThickness="0" Background="{x:Null}"/>
                </StackPanel>
            </StackPanel>
        </Border>
    </Grid>
   

5. Отображаем окно и инициализируем ViewModel.

   • Идем в App.xaml и удаляем там StartupUri="MainWindow.xaml"

   • Далее в App.xaml.cs переопределяе OnStartup:

       protected async override void OnStartup(StartupEventArgs e)
       {
           base.OnStartup(e);
       var viewModel = new MainViewModel();
       new MainWindow() { DataContext = viewModel }.Show();
       await viewModel.StartAsync();
     
     }
     

Собственно, на этом все, программа будет раз в 15 сек брать с сервера изображение и отображать нам его на экране

Вам остается эту все понять и подстроить под себя)