1

Только обнаружил большой кусок мусорного кода в приведённой ниже ViewModel:

class ManageStaffDataWindowViewModel : BasicViewModel {

    private StaffRepository staffRepository;
    public ObservableCollection<PersonEntity> staff { get; } =
        new ObservableCollection<PersonEntity>();
    private PersonEntity selectedPerson;

    public ManageStaffDataWindowViewModel() {
        staffRepository = new StaffRepository();
        staff = staffRepository.GetAllStaff();
    }

    public PersonEntity SelectedPerson {
        get { return selectedPerson; }
        set {
            selectedPerson = value;
            OnPropertyChanged("selectedPerson");
        }
    }

    private string _personStringifiedId;
    public string personStringifiedId {
        get { return _personStringifiedId; }
        set {
            _personStringifiedId = value;
            OnPropertyChanged("personStringifiedId");
        }
    }

    private string _personFullName;
    public string personFullName {
        get { return _personFullName; }
        set {
            _personFullName = value;
            OnPropertyChanged("personFullName");
        }
    }


    private RelayCommand addNewPersonCommand;
    public RelayCommand AddNewPersonCommand {
        get {
            return addNewPersonCommand ??
                (addNewPersonCommand = new RelayCommand(obj => {
                    AddNewPerson();
                }));
        }
    }

    private void AddNewPerson() {

        staff.Add(new PersonEntity() {
            PersonStringifiedId = personStringifiedId,
            PersonFullName = personFullName
        });

        staffRepository.UpdateDataFile(staff);
    }
}

В PersonEntity, то есть модели, уже объявлено:

public string PersonStringifiedId {
    get { return _personStringifiedId; }
    set {
        _personStringifiedId = value;
        OnPropertyChanged("PersonStringifiedId");
    }
}

public string PersonFullName {
    get { return _personFullName; }
    set {
        _personFullName = value;
        OnPropertyChanged("PersonFullName");
    }
}

Я сделал вывод, что во ViewModel похожие объявления лишние, когда удалил их вместе с командой AddNewPerson() и всё скомпилировалось. Таким образом, во ViewModel значения personStringifiedId и personFullName нам нужны только при добавлении в коллекцию нового экземпляра. Но разве мы не можем получить эти данные из делегированного объекта obj? Если можем, то как это сделать?


Обновление: View

<!-- Отображение имеющихся данных -->
<ListView ItemsSource="{Binding Path=staff}">
    <ListView.View>
        <GridView>
            <!-- ... -->
            <GridViewColumn DisplayMemberBinding="{Binding PersonStringifiedId, UpdateSourceTrigger=PropertyChanged}">
            <GridViewColumn DisplayMemberBinding="{Binding PersonFullName, UpdateSourceTrigger=PropertyChanged}" />
    <!-- ... -->


<!-- Ввод новых  данных -->
<TextBox>
    <TextBox.Text>
        <Binding Path="personStringifiedId"/>
    </TextBox.Text>
</TextBox>

<TextBox>
    <TextBox.Text>
        <Binding Path="personFullName"/>
    </TextBox.Text>
</TextBox>

<Button Command="{Binding addNewPersonCommand}"/>
1
  • 1
    Врапперы-вьюмодели над сущностями это нормально. Конечно WPF может биндиться напрямую и на вложенную модель. Но если у вас не только чтение. то могут быть проблемы. Если сразу писать в обернутую сущность, то при изменении свойства она может быть невалидна (зависит от стиля разработки конечно), но уже послать события INPC всем, кто следит. Для решения этого делают клонирование сущности и оборачивают ее или же создают чистые свойства во вьюмодели без завязки на вложенную сущность.
    – vitidev
    11 окт 2017 в 14:09

2 ответа 2

2

У вас какой-то странный вывод. То, что модельный объект реализует INPC, не означает, что VM-объект не должен его также реализовать. Если вы уберёте реализацию INPC из VM, то View не будет получать информацию об изменении в VM.

Вы, разумеется, можете спроксировать данные из модельного объекта. Но такая простая логика будет работать только в том тривиальном случае, если модельный объект по структуре полностью дублирует VM-объект, а также живёт в том же потоке. Если ваша модель общается с базой данных, это скорее всего не так, общение с базой данных не имеет права происходить в непонятно каком (а следовательно, в главном) потоке.


VM реализуется отдельно от модели. Модель имеет право меняться, иметь другую структуру данных и другое разбиение на классы, бежать в любом потоке, что угодно. Если у вас модель полностью дублирует VM и может бежать в основном потоке, возможно, в вашем проекте полноценный паттерн MVVM покамест не нужен. Но если вы пользуетесь базой данных, то доступ к ней в UI-потоке скорее всего является грубой ошибкой дизайна приложения.

6
  • Благодарю Вас за ответ! Прежде всего, позвольте узнать: что такое INPC? Явно не International Nuclear Physics Conference - первый результат поиска Google. 12 окт 2017 в 9:04
  • @GurebuBokofu: Это INotifyPropertyChanged :) Обычно всем лень выписывать полностью.
    – VladD
    12 окт 2017 в 9:13
  • Понял! :) Теперь хочу узнать, что Вы имеете ввиду под "реализацией INPC". Это объявление свойств с OnPropertyChanged("personFullName"); в Setter-е? 12 окт 2017 в 9:28
  • @GurebuBokofu: Ну, вы должны при каждом изменении свойства отправлять событие PropertyChanged. То есть не просто объявить событие, а и фактически его отправлять. Обычно это делается, да, через метод наподобие OnPropertyChanged(имя свойства)
    – VladD
    12 окт 2017 в 10:09
  • Хорошо, ещё раз спасибо Вам за объяснения! 12 окт 2017 в 23:45
2

Предисловие: Судя по всему, этот код вам достался в наследство.

Тут нельзя строго судить, является ли этот код мусорным, потому что такой подход используется, если Model активна.

в MVVM Model ничего не должна знать о VM, но что если извне кто-то изменил Model? А об этом нужно оповестить VM и View. Но при этом Model ничего не знает про VM. Для этого Model может реализовать INPC, а VM подпишется на поля Model, как View на поля VM.

Тем самым решается вопрос обновлении Model в две стороны.

Если у вас Model активна, то такой подход приемлем(хотя по моему мнению не является лучшим и является дурным тоном). Проблемы начинаются, когда VM и Model имеют сильно разную структуру(как и писал VladD)

Мой Ответ: Не узнав логики, которую заложил программист в этот код, вы не узнаете, мусорный ли это код или нет.

P.S. Вам также может помочь и проблема, с которой столкнулся я. Синхронизация Model и ViewModel, когда приложение - Client

UPD 1. Про соглашение.

Я не встречал соглашения по этому поводу, поэтому я бы на вашем месте предложил бы себе несколько способов, вот 2 из них.

  1. Использовать конструкцию типа

Я не встречал соглашения по этому поводу, поэтому я бы на вашем месте предложил бы себе на выбор 2 способа.

  1. Использовать конструкцию типа

    private void OnPropertyChanged([CallerMemberName] string propertyName = null)
        {
            var handler = PropertyChanged;
            if (handler != null)
            {
                handler(this, new PropertyChangedEventArgs(propertyName + "VM"));
            }
        }
    

В Model

private void OnPropertyChanged([CallerMemberName] string propertyName = null)
            {
                var handler = PropertyChanged;
                if (handler != null)
                {
                    handler(this, new PropertyChangedEventArgs(propertyName + "Model"));
                }
            }

Фишка(допустим в VM) в этой строке

handler(this, new PropertyChangedEventArgs(propertyName + "VM"));

В чем суть. вы обзываете ваши свойства одинаково и в Model и в VM, но привязанные свойства различаются идентификатором в конце. Вызов происходит просто

OnPropertyChanged();

Туда подсовывается имя свойства(метода), вызвавшего его.

Но вообще мне уже не нравится этот подход. При маштабировании(допустим будет поле для изменения). Так же он называтся не может, поэтому способ два.

  1. Создаете PersonViewModel , который повторяет Model. Добавляете команду добавить пользователя Далее в XAMl создаете Grid, DataContext которого является PersonViewModel(или AddPersonViewModel. В нем создаете 2 TextBox и кнопку для добавления новой персоны.

    <Grid DataContext="">
        <TextBox></TextBox>
        <TextBox></TextBox>
        <Button></Button>
    </Grid>
    

Далее связал бы ManageStaff с PersonViewModel(хотя подписаться на выполнение команды). Выглядит лаконично, хорошо маштабируемо(в том числе и названия). Теперь вы можете сделать свой UserControl, который будет в себе содержать структуру выше и добавлять куда ходите. Из-за того, что ManageStaff подписался на событие команды, а PersonViewModel юзает одну и ту же команду...Не важно, куда вы засунете ваш UserControl, ManageStaff получит изменения(если существует). Дело в том, что ManageStaff по своей логике управляет персоналом, но вполне так же можно отделить от него, создав поле в PersonViewModel статическое поле(или синглтон, если хочется по красоте) Persons и дать ссылку ManageStaff. Чтобы логично это выглядело, можно объединить в одну папку по управлению персоналом.

Я решил, что вам будет интересно услышать о еще некоторых способах сделать то, что вы хотите. На самом деле разделять ManageStaff как душе угодно, уже тут ваше дело и дело вкуса. Обычно очень помогает при построении ViewModel помнить про буковку S из SOLID.

UPD 2. Важный момент способа 1. Способ 3.

Посмотрите про привязку, как она происходит. Если вкратце, то дочерний элемент, биндится к свойству родительского элемента, но дочка глупая и гибкая. Ей важно только то, что имена свойств похожи. Так что если вы правильно сделаете, то они и обзываться будут одинаково, но я бы отделил, ибо при разрастании начал бы путаться.

13
  • Не знаю, можно ли сказать, что мне этот код достался по наследству. Мне, как разработчику без опыта в MVVM, нужно было взять что-то за основу, и я взял этот простейший пример. Но там, как видите, в VM нет свойств, повторяющих свойства Model (Phone). Что касается появления этого возможно мусорного кода, то это не иначе как я сделал по образцу другого примера без понимания, а теперь чего-то начал понимать и задался вопросом: а нужно ли повторять свойства Model в VM? Условно отвечу на свой вопрос: если это ни на что не повлияет, то не нужно. 12 окт 2017 в 9:00
  • Ну, у вас отличается код. Там это обязательно, потому что Binding происходит напрямую на Model(в некоторых редких случаях, когда приложения маленькое и допустим без идей на расширение, это допустимо). Там код модели не повторяется в VM, а просто View байндится на Model. Тобишь Model и VM работают как VM, просто идет разделение на поля Model(тобишь поля конкретно модели) и VM(тобишь поля View). В вашем случае вы повторяете свойства Model, поэтому INPC можно из Model вообще убрать(я только щас заметил, что вы в VM не привязываетесь к INPC вашей Model)
    – Arantler
    12 окт 2017 в 9:19
  • @GurebuBokofu и да, vitidev к вопросу добавил комментарий, также отвечающий на данный вопрос и говорит то же самое, только отношение к этому другое и раскрывает дополнительные варианты. Но это уже глубоко холиварно-философские вопросы.
    – Arantler
    12 окт 2017 в 9:23
  • Так, хорошо, пока всё ясно. Спрошу так: а если я из модели удалять INPC не буду, а из VM уберу personFullName и personStringifiedId, то что произойдёт? Я уже установил, что ошибки компиляции не будет, если убрать также и AddNewPerson. 12 окт 2017 в 9:24
  • @GurebuBokofu ну, во первых если вы INPC везде удалите, у вас все равно скомпилируется, ибо в WPF не все ошибки вылазят при компиляции(не равняйтесь на это). Чтобы ответить на вопрос, нужно показать кусок View. Там происходит сначала привязка к Model <ListBox Grid.Column="0" ItemsSource="{Binding Phones}" SelectedItem="{Binding SelectedPhone}"> А коллекция содержит Phone(модели короче). А потом идет привязка к его объектам. Нужен короче View ваш.
    – Arantler
    12 окт 2017 в 9:27

Ваш ответ

Нажимая на кнопку «Отправить ответ», вы соглашаетесь с нашими условиями использования и подтверждаете, что прочитали и поняли наши политику конфиденциальности и нормы поведения.

Всё ещё ищете ответ? Посмотрите другие вопросы с метками или задайте свой вопрос.