trjhtr

I've got a little application that I've dumbed down to show you the basics of what an excel reader would look like for new foods, consisting of Fruits and Meats.

The Application is written in WPF, C# and follows the MVVM architecture.
So on my main window I have a Create button, that I want to set disabled if something from both lists isn't valid. These lists are resided inside a user control that hosts both lists in 2 data grids.

How I currently handle this is that I let the OnPropertyChanged event from my objects trigger an event for the entire lists, that triggers an event for the entire user control that triggers an event for the main window which then checks if all sub items are valid.

This works and lets me very easily expand this check without using duplicate data.

However I'm not sure that events triggering other events in a tree, especially this big, is considered 'Good Practice'. So that's why I thought of asking some help from the code gods.

You can see dumbed down version of what I'm currently doing bellow.

Main Window

public void Init(ImportType type)

 var view = new FoodView();
 SelectedView = view; 
 SelectedView.CollectionItemChanged += (s, e) => RaisePropertyChanged(nameof(CheckErrors));

User control food

public class FoodViewModel : ObservableObject, IImporter

 public event PropertyChangedEventHandler CollectionItemChanged;
 private FoodFileProcessor rfp;

 public void SetFileAsync(string fullFilePath)
 
 rfp.FoodCollectionItemChanged += (s, e) => OnCollectionItemChanged(s, e);

 // Open and process the file passed
 Task.Run(() => rfp.OpenFile(fullFilePath));
 

File processing

public class MeatFileProcessor

 public event PropertyChangedEventHandler MeatsCollectionItemChanged;

 public ObservableCollection<Fruit> NewFruits = new ObservableCollection<Fruit>();
 public ObservableCollection<Meat> NewMeats = new ObservableCollection<Meat>();

 public void OpenFile(string filePath)
 
 using (var xlApp = new ExcelPackage(new FileInfo(filePath)))
 
 FruitTabProcessor.ListItemProperyChanged += (s, e) => OnFruitsCollectionChanged(s, e); ;
 ObservableCollection<Fruit> fruits = FruitTabProcessor.FormatFruit(xlApp.Workbook.Worksheets[2]);
 NewFruits = fruits;

 MeatTabProcessor.ListItemProperyChanged += (s, e) => OnMeatsCollectionChanged(s, e); ;
 var meats = MeatTabProcessor.FormatMeat(xlApp.Workbook.Worksheets[1]);
 NewMeats = meats;
 
 
 
}

This is then an example of where a food object would get created, for example meat.

private static void NewMeat(Meat meat)

 meat.PropertyChanged += Meat_PropertyChanged;


private static void Meat_PropertyChanged(object sender, PropertyChangedEventArgs e)

 OnListItemPropertyChanged(sender, e);


protected static void OnListItemPropertyChanged(object s, PropertyChangedEventArgs e)

 ListItemProperyChanged?.Invoke(s, e);

I was wondering if this is fine or if there is a better way of handling this situation? Is it OK to have events trigger other events because this kind of feels like a bad thing to do. I've read about the dangers of getting in a deadlock but seeing that this doesn't really do anything other then notifying change, this wouldn't be an issue.

Couple of comments.

Look into ICommand, I use this nuget package https://www.nuget.org/packages/RelayCommand

This allows you to:

public ICommand SomeCommand => new RelayCommand<SomeType>(DoSomething, x => CanDoSomething());

public void DoSomething(SomeType parameter)

 //ToDo: Implement me


public bool CanDoSomething()

 //ToDo: Implement me

When you can't do something (Items not selected for example) then your button will disable automatically.

Couple notes on ObservableCollection. This will notify you when an item is added.

The object that you add and remove need to implement INotifyProperty changed so you can monitor changes in your view to your objects.

You are using alot of event handler subscriptions - but you aren't unsubscribing - this may lead to memory leaks. Leverage ICommand where possible to avoid leaks.

I'm not sure if this is correct of not but your FruitTabProcessor Property changed handler is subscribing to OnMeats shouldn't it subscribe to OnFruits?