C# WPF ComboBox not displaying 'naziv' field values of 'Skola' class - what am I doing wrong with the data-binding?

Question

C# WPF, i am trying to bind field 'name' (naziv) of the class 'school' (skola) to ComboBox but it never shows names of the schools in ComboBox...

this is school class:

internal class Skola : INotifyPropertyChanged
    {
        public event PropertyChangedEventHandler PropertyChanged;
        private int ID;
        private string naziv;
        private string adresa;
        private string slika;
        private ObservableCollection<Ucenik> ucenici;

        public Skola() { }
        public Skola(int ID, string naziv, string adresa, string slika)
        {
            this.ID = ID;
            this.naziv = naziv;
            this.adresa = adresa;
            this.slika = slika;
            ucenici = new ObservableCollection<Ucenik>();
        }
public string Naziv
        {
            get { return naziv; }
            set
            {
                if (naziv != value)
                {
                    naziv = value;
                    NotifyPropertyChanged("Naziv");
                }
            }
        }

this is main:

public partial class MainWindow : Window
    {
        private ObservableCollection<Skola> skole;
        private Skola NEUPISANI = new Skola(0, "", "", "");
        private Skola selected_skolice = new Skola();

        public MainWindow()
        {
            InitializeComponent();
            skole = new ObservableCollection<Skola>();
            Skola s1 = new Skola(1, "Jozef Atila", "Sarplaninska 8", "");
            Skola s2 = new Skola(2, "Bogdana Saputa", "Nardonog fronta 14", "");

            s1.dodajUcenikaTest(new Ucenik("0021233213", "Milan", "Reljin", "Radnicka 12", ""));
            s2.dodajUcenikaTest(new Ucenik("034321233213", "Dragan", "Markovic", "Crnacka 33", ""));

            skole.Add(s1);
            skole.Add(s2);
            skole.Add(NEUPISANI);
            //skolice.DisplayMemberPath = "Naziv";

            DataContext = this;
        }

this is ComboBox in XAML:

        <ComboBox x:Name="skolice" Margin="180 10 180 30" Grid.ColumnSpan="2"
ItemsSource="{Binding Path=skole}" SelectionChanged="skolice_SelectionChanged"></ComboBox>

p.s. if it's too confusing that not everything is in english, I'll change it!

score 0 · Answer 1 · answered May 31 '23 at 19:04

0

The "skole" field you are binding to is private. Wrap it in a property and bind to the property :

public IEnumerable<Skola> Skole
{
    get { return this.skole; }
}

By the way, there is a bunch of bad practices in your code - it's not the direct issue you are facing, but it can make less effective to debug/show to people etc.

answered May 31 '23 at 19:04

Romka

144
8

should i put this in main or? because it won't let me put it there – StraleS02 May 31 '23 at 19:10
1

In mainwindow, but it would be better to have a viewmodel for the mainwindow too, to adhere to the MVVM pattern – Philip Stuyck May 31 '23 at 19:12
Yes, it will be better, as Philip said, to create a class - let's say "MainViewModel", put the field and the property into it and make it the MainWindow.DataContext. But, it's likely not the issue you mention ("it won't let me put in there") ; I don't have enough information to be sure but maybe you have to add "using System.Collections.Generic" – Romka May 31 '23 at 19:35
The point is not that the field is *private*. the point is that the binding path points to a *field*. **In WPF the binding source must be a public property**. You can't bind to fields or methods in WPF. – Jun 01 '23 at 09:28
Please read my comments [here](https://stackoverflow.com/a/76376588/21970328) to understand why the property should be of type INotifyCollectionChanged instead of IEnumerable. – Jun 01 '23 at 09:30
For reasons of performance, all properties you use as a binding source (e.g. Skole) must be dependency properties (because the OP implements this property in the `Window`, which is a `DependencyObject`. Otherwise, if the source is not a `DependencyObject`, the source must implement `INotifyPropertyChanged`). – Jun 01 '23 at 09:33
I mention this because you were talking about bad practices. – Jun 01 '23 at 09:35
@BionicCode. when you state that "For reasons of performance, all properties you use as a binding source (e.g. Skole) must be dependency properties", you mean, if I understand correctly, that MVVM is a bad practice, am I right ? And, basing your assertion about performance, could you please provide a reproductible downladable test where inherhiting from DependencyProperty, staying in the context of the current post, will provide significant, user-XP measurable, better experience (in a theoricle, non-reproductible real-world way, ignoring maintainability, testable way, of course) ? – Romka Jun 01 '23 at 12:48
"Please read my comments here to understand why the property should be of type INotifyCollectionChanged instead of IEnumerable." please excuse me if I am getting wrong, but about my humble 15 years XP in wpf I discovered that the context has to implement INotifyPropertyChanged,not the properties ? Or, if you are writing about the collection itslef, then your comment makes sense outside of the current post, but in the current case I think we all understood that the collection will never be updated, or, whether it will, that is out of the current questoin's scope – Romka Jun 01 '23 at 12:54
*"if I understand correctly, that MVVM is a bad practice"* - Not sure how you get this conclusion out of my explanation. I didn't even mention MVVM as the issue is not MVVM related. It's related to how the binding engine works. Performance costs occur when resolving the binding source. No dependency property and no INotifyPropertyChanged (no change notification at all) performs the worst as the engine has to use heavy reflection to obtain the property descriptor in order to subscribe to the ValueChanged event. – Jun 01 '23 at 15:09
The related handler is stored in a static reference table which will result in the binding source being rooted (root object aka memory root): the GC can't collect the binding source and it's reference tree, therefore creating a memory leak. Implementing INotifyPropertyChanged does not rely that heavy on reflection. The engine casts the object to INotifyPropertyChanged to subscribe to PropertyChanged event. Then it uses little reflection to resolve the property path on the binding source. – Jun 01 '23 at 15:09
Dependency properties don't require reflection at all. They are managed by the dependency property system. This makes dependency properties the most performant binding source. The difference is measurable and relevant in a big application with a complex UI that heavily makes use of data binding. That'S why you should consider to use BindingMode.OneTime where possible as this will prevent the binding engine to subscribe to any change notification. For example UWP makes OneTime the default mode - to improve application performance. – Jun 01 '23 at 15:09
*"but about my humble 15 years XP in wpf I discovered that the context has to implement INotifyPropertyChanged,not the properties" - that's correct. As explained in my previous comment, if the binding source does not implement this interface and the source property is not a dependency property, then massive costs occur. Aside from the heavy reflection you are essentially creating a memory leak because the binding source becomes a root object (static reference to the change handler). – Jun 01 '23 at 15:10
That's why your source object must implement the interface whether it raises the event or not. Just to prevent the binding engine to create the mentioned static reference. Alternatively use BindingMode.OneTime to stop the binding engine from trying to track data changes. – Jun 01 '23 at 15:11
Regarding the collection, the collection used as a binding source must implement INotifyCollectionChanged in order to prevent the binding engine to create a strong reference that will keep the collection and all it's items and all references of those items (i.e. the complete reference tree) alive for a very long time potentially consuming a huge amount of memory. If you don't know this you and your customer will misinterpret this as a severe memory leak (if the objects are big enough to make the memory consumption become relevant). – Jun 01 '23 at 15:11
However, this is the case whether the collection is getting updated or never will. The binding engine does not know if the data object is a candidate for changes or not. It will always try to track the data objects for changes. If you know the data won't change you tell this to the binding engine by configuring the data binding as OneTime. That's the whole point. Whether properties change or not, the source must implement INotifyPropertyChanged or the relevant properties as dependency properties or configure the Binding as OneTime. – Jun 01 '23 at 15:11
Whether the collection will change or not, it must implement INotifyCollectionChanged to allow all related instance to be eligible for the GC as soon as possible (to allow to manage memory more efficiently). – Jun 01 '23 at 15:11
I missed the OneWayToSource BindingMode which will also prevent the binding engine from observing the data source for changes. – Jun 01 '23 at 15:16
@StraleS02 if my answer helped you,please flag it. Dispite the tons of logorrhea after it. – Romka Jun 08 '23 at 12:18

Philip Stuyck · Answer 2 · 2023-06-02T05:44:18.043

0

As it is implemented right now there are multiple problems. For one you have to make a public property of the skole field. If you do that the combobox will show the type name of the class holding all data so you would see several "skola" items in the list. To solve this you have to set some properties on the combobox :

<ComboBox ItemsSource="{Binding Path=skola}"
DisplayMemberPath="naziv"/>

edited Jun 02 '23 at 05:44

answered May 31 '23 at 19:09

Philip Stuyck

7,344
3
28
39

1

If I understod correctly - and also because I understand a bit of some slavic languages - "Skola" is singular and "Skole" is plural, so the property in your sample would be "Skole". Furthermore, "naziv" is the name of the backing private field, it will not be accessed by the DisplayMemberPath. It has to be "Naziv" – Romka May 31 '23 at 20:16
When binding to a collection or calling `CollectionViewSource.GetDefaultView()`, the binding engine will use the `ICollectionView` of this collection as source. It keeps a weak reference to this view in a table in order to use it with the registered `Binding`. The reference is also stored to allow `CollectionViewSource.GetDefaultView()` to return the same `CollectionView` instance to every caller. The secondary goal is the lifetime management of the underlying collection. The engine will keep it alive as long the associated `ICollectionView` is referenced. – Jun 01 '23 at 09:22
When possible, this is done implicitly, because the `IColectionView` always tries to listen to a `CollectionChanged` event in order to stay in sync. This creates a strong reference from the underlying collection (event source) to the `ICollectionView` (event listener). This strong reference ensures that the collection can never get garbage collected before the `ICollectionView`. – Jun 01 '23 at 09:23
Now, if this underlying collection does not implement `INotifyCollectionChanged`, the engine will be forced to create a strong reference to the underlying collection by itself (because in this case the `ICollectionView` can't observe the underlying collection). This means, where in the first case the owner of the collection was in full control of the collection's lifetime, the lifetime management is now the responsibility of the binding engine. – Jun 01 '23 at 09:23
To prevent a leak introduced by the new strong reference, the engine will periodically try to purge the reference table in order to end the lifetime of the collection. For efficiency, the algorithm will complete a purge cycle every now and then which will result in the collection(s) to stay alive in memory for a very long time after they are no longer used by the client code (which is commonly and wrongfully perceived as a memory leak). That's why when binding to collections, the source collection must always implement `INotifyCollectionChanged` - even when there won't be changes. – Jun 01 '23 at 09:23
The overhead introduced by the event handling is negligible in contrast to the memory costs if the interface is not implemented. A similar general rule for binding to source objects exist too: when binding to an object, the source object must always implement `INotifyPropertyChanged` (even if there are no changes) or implement the source properties as dependency properties. – Jun 01 '23 at 09:24
This means: unless the property that holds the collection value is not a dependency property, the owner of this property that exposes the collection (that must implement `INotifyCollectionChanged` even when the collection won't change) for data binding must implement `INotifyPropertyChanged` too (even when the property won't change - to prevent a *real* memory leak). – Jun 01 '23 at 09:24
This explains while your following statement is a bad recommendation: *"Btw the observablecollection is intented to be used in cases where you expect that the content of the list is subject to changes while you are showing it. If the list is fixed, you don't have to use an observablecollection."*. Following this recommendation leads to badly performing code. the problem gets worse the more such bindings exists and the larger the collection are. – Jun 01 '23 at 09:25
Thank you for your thorough explanation, I removed the comment. Goes to show I can improve my own code. – Philip Stuyck Jun 02 '23 at 05:46

C# WPF ComboBox not displaying 'naziv' field values of 'Skola' class - what am I doing wrong with the data-binding?

2 Answers2