4

I am working on a WPF app using the MVVM patterm, which I am learning. It uses EF4. I am trying to use a similar tabbed document interface style; several combo boxes on these tabs have the same items sources (from a sql db). Since this data almost never changes, it seemed like a good idea to make a repository object to get them when the app starts, and just reuse them for each viewmodel. For whatever reason though, even though I use new in the constructors, the lists are connected.

If I set a bound combo box on one tab, it gets set on another (or set when a new tab is created). I don't want this to happen, but I don't know why does.

The repository object is initialized before anything else, and just holds public lists. The views simply use items source binding onto the ObservableCollection. I am using the ViewModelBase class from the article. Here is the Viewmodel and model code.

ViewModel

TicketModel _ticket;

    public TicketViewModel(TableRepository repository)
    {
        _ticket = new TicketModel(repository);
    }

    public ObservableCollection<Customer> CustomerList
    {
        get { return _ticket.CustomerList; }
        set
        {
            if (value == _ticket.CustomerList)
                return;

            _ticket.CustomerList = value;

            //base.OnPropertyChanged("CustomerList");
        }
    }

Model

public ObservableCollection<Customer> CustomerList { get; set; }

    public TicketModel(TableRepository repository)
    {
        CustomerList = new ObservableCollection<Customer>(repository.Customers);
    }

EDIT: I am sure this is the wrong way to do this, I am still working on it. Here is the new model code:

        public TicketModel(TableRepository repository)
    {
        CustomerList = new ObservableCollection<Customer>((from x in repository.Customers
                                                           select
                                                               new Customer
                                                               {
                                                                   CM_CUSTOMER_ID = x.CM_CUSTOMER_ID,
                                                                   CM_FULL_NAME = x.CM_FULL_NAME,
                                                                   CM_COMPANY_ID = x.CM_COMPANY_ID
                                                               }).ToList());
    }

This causes a new problem. Whenever you change tabs, the selection on the combo box is cleared.

MORE EDITS: This question I ran into when uses Rachels answer indicates that a static repository is bad practice because it leaves the DB connection open for the life of the program. I confirmed a connection remains open, but it looks like one remains open for non-static classes too. Here is the repository code:

using (BT8_Entity db = new BT8_Entity())
        {
            _companies = (from x in db.Companies where x.CO_INACTIVE == 0 select x).ToList();
            _customers = (from x in db.Customers where x.CM_INACTIVE == 0 orderby x.CM_FULL_NAME select x).ToList();
            _locations = (from x in db.Locations where x.LC_INACTIVE == 0 select x).ToList();
            _departments = (from x in db.Departments where x.DP_INACTIVE == 0 select x).ToList();
            _users = (from x in db.Users where x.US_INACTIVE == 0 select x).ToList();
        }

        _companies.Add(new Company { CO_COMPANY_ID = 0, CO_COMPANY_NAME = "" });
        _companies.OrderBy(x => x.CO_COMPANY_NAME);

        _departments.Add(new Department { DP_DEPARTMENT_ID = 0, DP_DEPARTMENT_NAME = "" });
        _locations.Add(new Location { LC_LOCATION_ID = 0, LC_LOCATION_NAME = "" });

However, now I am back to the ugly code above which does not seem a good solution to copying the collection, as the Customer object needs to be manually recreated property by property in any code that needs its. It seems like this should be a very common thing to do, re-using lists, I feel like it should have a solution.

Community
  • 1
  • 1
Kyeotic
  • 19,697
  • 10
  • 71
  • 128
  • Whats wrong with loading your static lists once, then closing the connection? The static class of lists is NOT a repository since it isn't used to add/edit/delete items, it's simply a class full of some static lists. – Rachel Aug 09 '11 at 03:21
  • Because it seems that even when using a `using` block, a connection to the db remains open. Unless there is an explicit way to make EF close its connection, then this is not a good practice. – Kyeotic Aug 09 '11 at 14:37
  • In the past I've used a `using` block for my DataContext and returned `context.Entities.ToList()` to the Static Lists. This disconnects the entities from the context and closed the connection. So my Static Class would say `CustomerList = DAL.GetCustomers();` and the static `GetCustomers()` method on my `DAL` class would be something like `List list; using (var context as new MyEntities) { list = context.Customers.ToList(); } return list;` – Rachel Aug 09 '11 at 14:45

1 Answers1

4

Custom objects, such as Customer get passed around by reference, not value. So even though you're creating a new ObservableCollection, it is still filled with the Customer objects that exist in your Repository. To make a truly new collection you'll need to create a new copy of each Customer object for your collection.

If you are creating multiple copies of the CustomerList because you want to filter the collection depending on your needs, you can use a CollectionViewSource instead of an ObservableCollection. This allows you to return a filtered view of a collection instead of the full collection itself.

EDIT

If not, have you considered using a static list for your ComboBox items, and just storing the SelectedItem in your model?

For example,

<ComboBox ItemsSource="{Binding Source={x:Static local:Lists.CustomerList}}"
          SelectedItem="{Binding Customer}" />

This would fill the ComboBox with the ObservableCollection<Customer> CustomerList property that is found on the Static class Lists, and would bind the SelectedItem to the Model.Customer property

If the SelectedItem does not directly reference an item in the ComboBox's ItemsSource, you need to overwrite the Equals() of the item class to make the two values equal the same if their values are the same. Otherwise, it will compare the hash code of the two objects and decide that the two objects are not equal, even if the data they contain are the same. As an alternative, you can also bind SelectedValue and SelectedValuePath properties on the ComboBox instead of SelectedItem.

Rachel
  • 130,264
  • 66
  • 304
  • 490
  • What is the best way to create a new copy of an object coming from EF? – Kyeotic Aug 08 '11 at 17:11
  • @Tyrsius Simplest way would be to create a new object and set it's properties equal to the copied object. Why do you want copies anyways? – Rachel Aug 08 '11 at 17:47
  • I do not want the lists to be linked. Each tab is going to be indepdendant, so making a selection on one should not cause all other comboboxes to show the same selection. – Kyeotic Aug 08 '11 at 17:53
  • Setting the properties seems like a clumsy solutions. If I need to create a copy in more than one place, I have to copy that bit of code. Since its coming from EF I cannot make a constructor to do this. Is there no other way? You answered my original question, maybe I should start a new question for this. – Kyeotic Aug 08 '11 at 18:12
  • @Tyrsius See my edit. In this situation, your model should only store the `SelectedItem`, not the entire item list. Generally I use a static class to store my lists of objects, although the list of objects can also be located in a ViewModel. – Rachel Aug 08 '11 at 18:12
  • This looks promising, but I do not entirely understand it. Will the selectedItem then just be an int property? – Kyeotic Aug 08 '11 at 18:35
  • 1
    `SelectedItem` is the data object that is selected and it is of whatever type your ComboBox's `ItemSource` collection is. `SelectedValue` is whatever the `Value` of your SelectedItem is (set via `SelectedValuePath` property), and `SelectedIndex` is the integer index of the selected item. – Rachel Aug 08 '11 at 18:49
  • That explanation helped. However, I tried making a static class to hold the lists, but I cannot get the syntax for the itemsource to bind to it working. local looks like an xmnls you declared, but I get errors anytime I try what you put put in. – Kyeotic Aug 08 '11 at 18:56
  • 1
    The binding `{Binding Source={x:Static local:Lists.CustomerList}}` means it is looking in the namespace defined in `local` for a static class called `Lists` which has a static property called `CustomerList`. You must define the `local` yourself at the top of your XAML page using something like `xmlns:local="clr-namespace:MyNamespace"` – Rachel Aug 08 '11 at 19:13
  • I must have made a typo the first time I tried that. I think i've got it now. Thank you very much for all your help – Kyeotic Aug 08 '11 at 19:29