EntityView2 IBindingList differences to BindingList<T>

Posts   
1  /  2
 
    
yowl
User
Posts: 271
Joined: 11-Feb-2008
# Posted on: 29-Jun-2009 17:29:53   

I have 2 forms with DevExpress Grids (1 view with 1 column) and I am experimenting with replacing items in BindingList<T> and EntityView2 (EntityCollection<T>.DefaultView). This code with BindingList<T> does what I expect when I press the button, i.e. the text in the bottom cell changes to "c":

    private void Form1_Load(object sender, EventArgs e)
    {
        bl = new BindingList<Entity>();

        gridControl1.DataSource = bl;
        gridControl1.DataMember = "";

        bl.Add(new Entity("a"));
        bl.Add(new Entity("b"));

        bl.ListChanged += bl_ListChanged;
    }

    public static void bl_ListChanged(object sender, ListChangedEventArgs e)
    {
        Debug.WriteLine("ListChanged " + e.OldIndex + "," + e.NewIndex + " " + e.ListChangedType);
    }

    private void simpleButton1_Click(object sender, EventArgs e)
    {
        bl[1] = new Entity("c");
    }

And gives the output: ListChanged -1,1 ItemChanged

However this code does not, and produces no output (ListChanged does not fire):

    private void Form1_Load(object sender, EventArgs e)
    {
        vl = new EntityCollection<VoyageEntity>();

        gridControl1.DataSource = vl.DefaultView;
        gridControl1.DataMember = "";

        vl.Add(CreateVoyageEntity("a"));
        vl.Add(CreateVoyageEntity("b"));

        vl.DefaultView.ListChanged += Form1.bl_ListChanged;
    }

    private void simpleButton1_Click(object sender, EventArgs e)
    {
        vl[1] = CreateVoyageEntity("c");
    }

    static VoyageEntity CreateVoyageEntity(string s)
    {
        var v = new VoyageEntity {VoyageShipFullName = s};
        return v;
    }

Other code:

public class Entity : INotifyPropertyChanged
{
    public event PropertyChangedEventHandler PropertyChanged;

    public Entity(string name)
    {
        this.name = name;
    }

    string name;
    public string Name
    {
        get { return name; }
        set
        {
            if(name != value)
            {
                name = value;
                OnPropertyChanged();
            }
        }
    }

    void OnPropertyChanged()
    {
        if(PropertyChanged != null)
        {
            PropertyChanged(this, new PropertyChangedEventArgs("Name"));
        }
    }
}

I assume this is by design, so is there a way to make the EntityView behave in such a was as to make the grid realise one of its items has been replaced?

Thanks,

Scott

.Net 3.5 LLBLGen 2.6 Final (Adapter) DevExpress 9.1.4 SQL Server 2008 (but there is no data access here)

simmotech
User
Posts: 1024
Joined: 01-Feb-2006
# Posted on: 30-Jun-2009 06:23:10   

I have a feeling that this is the indexer setter in CollectionCore<T> raising the event with ListChangedType.ItemAdded.

I just tried changed yowl's code to add this after the change

    var mi = typeof(CollectionCore<VoyageEntity>).GetMethod("OnListChanged", BindingFlags.NonPublic | BindingFlags.Instance);
    mi.Invoke(vl, new object[] { 1, ListChangedType.ItemChanged });

and the grid now sees the change and updates the display.

Cheers Simon

simmotech
User
Posts: 1024
Joined: 01-Feb-2006
# Posted on: 30-Jun-2009 06:33:39   

And another thing....smile

There is a subtle difference in the way that BindingList<T> raises its ListChangedEvent: When the list item itself is replaced, it leaves the OldIndex parameter as -1; however, when the contents of a list item are replaced, it sets the OldIndex parameter to be the same as the NewIndex parameter.

This makes sense to me as it distinguishes between two different operations (though as usual, the MS documentation is crap) - maybe the OnEntityInListOnEntityContentsChanged could be changed to follow this 'convention'?

Cheers Simon

Walaa avatar
Walaa
Support Team
Posts: 14993
Joined: 21-Aug-2005
# Posted on: 30-Jun-2009 10:09:33   

I thjink you should use the ListChanged event of the entityCollection not the EntityView, would you please try to do that.

Instead of: vl.DefaultView.ListChanged += Form1.bl_ListChanged;

Try: vl.ListChanged += Form1.bl_ListChanged;

yowl
User
Posts: 271
Joined: 11-Feb-2008
# Posted on: 30-Jun-2009 10:36:47   

With ListChanged on the EntityCollection you get:

ListChanged -1,1 ItemAdded

But this does not help the grid (either DevExpress or Microsoft's DataGridView). Its my understanding that when binding to an EntityCollection the DefaultView property is called by the bound control anyway. So the bound control will not received the ItemAdded change and also its the wrong ListChangedType. From the documentation

Entity collections in LLBLGen Pro implement the IListSource interface. This means that bound controls will request from the interface an object they can bind to. Every entity collection returns its DefaultView when this request comes. This thus means that when you set a control's DataSource property to an entity collection instance, the collection won't bind directly to the control though it will bind to the control through its DefaultView.

Walaa avatar
Walaa
Support Team
Posts: 14993
Joined: 21-Aug-2005
# Posted on: 30-Jun-2009 12:00:14   

I might be mistaken, but I think your issue is related to the following threads: http://www.llblgen.com/TinyForum/Messages.aspx?ThreadID=8543 http://www.llblgen.com/TinyForum/Messages.aspx?ThreadID=8381

simmotech
User
Posts: 1024
Joined: 01-Feb-2006
# Posted on: 30-Jun-2009 12:37:26   

Whether a Grid's DataSource is set to EntityCollection<T> or EntityCollection<T>.DefaultView is irrelevant, the result is the same.

The problem is that EntityCollection<T> is firing ItemAdded whereas I believe it should be ItemChanged for the situation where a ListItem is replaced with another.

EntityViewBase<T> does not propagate any ListChanged event in this scenario.

I have proved this by listening to the ListChanged events fired by both the collection and the DefaultView:


    vl.DefaultView.ListChanged += bl_DefaultViewListChanged;
    vl.ListChanged += bl_ListChanged;

(where vl is the EntityCollection<T>)

I added a new button to do this:-

    private void simpleButton2_Click(object sender, EventArgs e)
    {
        vl[1] = CreateVoyageEntity("c");

        Debug.WriteLine("Now firing Changed event");

        var mi = typeof(CollectionCore<VoyageEntity>).GetMethod("OnListChanged", BindingFlags.NonPublic | BindingFlags.Instance);
        mi.Invoke(vl, new object[] { 1, ListChangedType.ItemChanged });
    }

The output is this:- ListChanged -1,1 ItemAdded Now firing Changed event DefaultViewListChanged -1,1 ItemChanged ListChanged -1,1 ItemChanged

Hence the ChangedEvent is propagated by both the View and the Collection whereas ItemAdded is blocked by the View.

Because the Collection is firing ItemAdded and the View blocking it, the Grid is not getting any indication of a change and so does not update.

Cheers Simon

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39859
Joined: 17-Aug-2003
# Posted on: 30-Jun-2009 13:10:11   

I think it's related to a feature wink http://www.llblgen.com/documentation/2.6/hh_goto.htm#Using%20the%20generated%20code/Adapter/gencode_usingentityview_adapter.htm%23viewbehavior

Could you try with a newly Created EntityView2 instance which has the proper behavior set?

Btw, the view has no business with the events of the collection, it has its own events.

Frans Bouma | Lead developer LLBLGen Pro
yowl
User
Posts: 271
Joined: 11-Feb-2008
# Posted on: 30-Jun-2009 13:13:06   

Thanks for the links. I dont think they are the same issue, but I tried the second one anyway with no success. The first is not relevant as it is about modifying the contents of an entity which I am not doing. I've attached a solution for your convenience. No DB is required. Start the application and press the replace button in each form. You will see that in the BindingList form the grid is updated, whereas in the EntityCollection form it is not.

Edit. I've removed all references to DevExpress to keep things simpler.

simmotech
User
Posts: 1024
Joined: 01-Feb-2006
# Posted on: 30-Jun-2009 13:51:07   

Otis wrote:

I think it's related to a feature wink http://www.llblgen.com/documentation/2.6/hh_goto.htm#Using%20the%20generated%20code/Adapter/gencode_usingentityview_adapter.htm%23viewbehavior

Could you try with a newly Created EntityView2 instance which has the proper behavior set?

Btw, the view has no business with the events of the collection, it has its own events.

I don't understand your last statement since this code is in the View:

    /// <summary>
    /// Binds the events of the related collection to own event handlers so the changes in the related collection are noted and handled here as well.
    /// </summary>
    private void BindEvents()
    {
        _relatedCollection.ListChanged += new ListChangedEventHandler( _relatedCollectionListChanged );
    }

I know it has its own events but it listens to the collection all the same to know what events of its own to send.

Have tried with

    gridControl1.DataSource = new EntityView2<VoyageEntity>(vl);

and all variants of DataChangeAction - exactly the same result.

Looking at HandleRelatedCollectionItemAdded in EntityViewBase<T> (which is where I think flow will go from _relatedCollectionListChanged), I think both isInsert and IsNewIndex will be false because _entityIndices will always contain the NewIndex and _entityIndices.Count == _relatedCollection.Count. Because isNewIndex == false, fireEvent stays false and so everything becomes a NOP at least where there is no filter and/or sorter - does this look likely?

I also don't think the DataChangeAction is relevant here. The default is ReapplyFilterAndSorter and we haven't changed that; there is no filter and no sorting.

I'll be blunt smile - why is ItemAdded being passed by the collection and not ItemChanged?

Cheers Simon

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39859
Joined: 17-Aug-2003
# Posted on: 30-Jun-2009 14:11:57   

I'll be briefly addressing both conversations below

In short: the view isn't re-sending the events of the collection 'as-is' as adding things to a collection doesn't necessarily mean it's added to the view and with a new filter, things might be added to the view but not the collection. I.o.w.: it's a different thing. BindingList and the entityview are also two different types of classes: the bindinglist is simply a collection which implements IBindingList and the EntityView is a view on a collection based on a filter.

So that both have different behavior, that's understandable, they're not the same thing.

The EntityView's IBindingList is solely focussed on what's in the view. There might be some tiny issues here, if so let's focus on these instead of focussing on what the difference between bindinglist and the view is as IMHO that's perhaps interesting but not a point we have to address to (i.o.w.: debate, but we won't participate).

simmotech wrote:

Otis wrote:

I think it's related to a feature wink http://www.llblgen.com/documentation/2.6/hh_goto.htm#Using%20the%20generated%20code/Adapter/gencode_usingentityview_adapter.htm%23viewbehavior

Could you try with a newly Created EntityView2 instance which has the proper behavior set?

Btw, the view has no business with the events of the collection, it has its own events.

I don't understand your last statement since this code is in the View:

    /// <summary>
    /// Binds the events of the related collection to own event handlers so the changes in the related collection are noted and handled here as well.
    /// </summary>
    private void BindEvents()
    {
        _relatedCollection.ListChanged += new ListChangedEventHandler( _relatedCollectionListChanged );
    }

I know it has its own events but it listens to the collection all the same to know what events of its own to send.

Of course it has to, if an entity is removed from the collection it has to know that. That's why it listens to it, but it doesn't resend them. I meant with that remark: the events aren't comparable: the collection raises events which concern the collection, the view monitors these and decides what to do based on what event is raised. It then either does nothing, or refilters itself, resets itself etc. and raises events based on that.

I'll be blunt smile - why is ItemAdded being passed by the collection and not ItemChanged?

What do you mean exactly?

For the record, I have a hard time finding out what this thread is all about: is it a bug? If so, where is it stated that behavior should be ABC and you get DEF? Or is it something that's confusing, perhaps due to misinterpretation? What does devexpress have to do with event raising from views btw?

Like: ItemAdded is send by the collection (line 1338, collectioncore), which is great, but of no value for the view directly: it won't resent the event just because it received it: the view is a view with a filter (which can be null), so it has to re-apply the filter (if applicable). if the item added matches the filter, an ItemAdded is raised from the view, but only then. if the sorter is applied as well, the order in which the items are stored in the view can be changed dramatically (i.e. the new item is at the top), so a reset is issued in that case instead of an item added.

Item changed is raised at line 1004. So again, I have no clue what this is all about.

So i.o.w.: please let 1 person explain what the problem is and we'll look at it. It's now unclear and confusing (at least to me). Also, leave control vendors out of this if possible, as it makes things only more complicated.

Frans Bouma | Lead developer LLBLGen Pro
yowl
User
Posts: 271
Joined: 11-Feb-2008
# Posted on: 30-Jun-2009 14:59:25   

I will try to be clearer. I have an EntityCollection which is bound to a grid (lets use MS DataGridView and forget DevExpress). One entity in that collection is being replaced and I want that change reflected in the grid. Thats it really.

I think the implementation of IBindingList is the relevant issue as that is what I think the grid is looking at. So I wanted to compare the grids behaviour to other implementations of IBindingList. I picked BindingList<T> as its the first class that came to mind where I could replace an element of the collection like I can with EntityCollection. I've also tried BindingSource on top of BindingList to get something closer semantically to EntityView2/EntityCollection. That solution is attached here.

Hope that helps.

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39859
Joined: 17-Aug-2003
# Posted on: 30-Jun-2009 14:59:57   

Btw, remember, LLBLGen Pro's entities raise their change event (which is republished as ListChanged by the collection and the view) AFTER EndEdit. Changing a property through code while you're on the row in the grid therefore might not be reflected after you move away from the row. The propertychanged event IS raised after a property is changed in an entity directly, however that's not influencing the view/collection to send ListChanged. There's a good reason for that: CancelEdit: which reverts the values but can't revert the effects of the event already raised.

This particular issue I described above is a debate I won't go into again, as it's been debated a couple of times already and it won't change.

As I said: BindingList is something else than the entity views.

Frans Bouma | Lead developer LLBLGen Pro
yowl
User
Posts: 271
Joined: 11-Feb-2008
# Posted on: 30-Jun-2009 15:02:20   

I seem to keep posting just before someone else disappointed . Please note that I am not changing any properties on any entity, I don't think EndEdit is relevant.

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39859
Joined: 17-Aug-2003
# Posted on: 30-Jun-2009 15:22:40   

yowl wrote:

I will try to be clearer. I have an EntityCollection which is bound to a grid (lets use MS DataGridView and forget DevExpress). One entity in that collection is being replaced and I want that change reflected in the grid. Thats it really.

I think the implementation of IBindingList is the relevant issue as that is what I think the grid is looking at. So I wanted to compare the grids behaviour to other implementations of IBindingList. I picked BindingList<T> as its the first class that came to mind where I could replace an element of the collection like I can with EntityCollection. I've also tried BindingSource on top of BindingList to get something closer semantically to EntityView2/EntityCollection. That solution is attached here. Hope that helps.

Ok.

There's a problem with the IBindingList.ListChanged event: if its raised and it has as argument ItemChanged, does that mean: 1) the original element was removed, a new one was added at the same index or 2) the element at spot X got its data changed

Unclear. The view however has to work with this event, as there's no other one implemented. The indexer setter raised itemadded, as that's what it can do from our point of view: ItemChanged would be ambiguistic. It's not the only flaw of ListChanged: removing an element also gives problems (you can't track which element was removed, the element is already removed when the event reaches subscribers).

I understand that ItemChanged might be a better choice in this case, however it gives a problem due to the ambiguistic behavior: the observer of the view has to perform an instance compare to check whether the object at the given index is indeed the same or it has changed. Is this always done? I have no idea, really simple_smile . According to the documentation of ListChangedType, if the argument is ItemChanged, the NewIndex is the index of the changed item. So what BindingList does (as Simon described above) is actually undocumented behavior: OldIndex is only used when ItemMoved is used. Nowhere is documented that OldIndex should be set when the contents of the item was changed to make ItemChanged less ambiguistic.

So either we'll do undocumented behavior as well (which I won't allow, as it always gets back to us at the worst possible moment) or we leave it as is and your code won't work properly which is also not ideal disappointed

Is it possible to change your code in such a way that when you click the button you remove the item at position X and add the new one? This would make the current code work as expected and your collection would contain the same elements.

Frans Bouma | Lead developer LLBLGen Pro
Otis avatar
Otis
LLBLGen Pro Team
Posts: 39859
Joined: 17-Aug-2003
# Posted on: 30-Jun-2009 15:37:02   

I looked at our upcoming v3.0 codebase which used our algorithmia library with neat collections which understand undo/redo etc. and I wondered why I didn't had the problem there as well. The thing: it first removes the element at the index (which raises with argument ItemRemoved) then it inserts the item at the index (which raises with argument ItemAdded).

Looking at the v2.x runtime code with that knowledge, it seems kind of logical to first raise ListChanged with ItemRemoved, and then ListChanged with ItemAdded, which would solve the problem without using undocumented crap like Bindinglist apparently does.

The problem with adding/changing events is that they can have devastating effects on existing code...

Frans Bouma | Lead developer LLBLGen Pro
yowl
User
Posts: 271
Joined: 11-Feb-2008
# Posted on: 30-Jun-2009 15:50:25   

Thanks for the reply. I agree its unclear, a replacement could be argued to be both a change and an add. Not sure where the discussions about OldIndex comes from, in all my tests with BindingList, BindingSource, EntityView2 it has always been -1 as described in the docs and I wouldn't suggest you set it to anything else.

Yes I can probably change the code, I can't see why removing the old entity, then adding the new entity will be a problem. I can also use reflection to fire the event again with ItemChanged as that works too.

You obviously have to decide whether to change your implementation or not. Thats a difficult decision. You have implemented IBindingList with ItemAdded in this scenario, something that is not clear you should do from the documentation, but then again some people may be relying on it. (I would argue they are relying on undocumented behaviour :-)).

In the exhaustive compatibility tests I've done with grids (heavy sarcasm here), I think LLBLGen would be better if it was changed. How about a switch for version 3 ;-) ?

yowl
User
Posts: 271
Joined: 11-Feb-2008
# Posted on: 30-Jun-2009 15:58:20   

Goddammit, you posted while I was typing again. simple_smile

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39859
Joined: 17-Aug-2003
# Posted on: 30-Jun-2009 17:12:59   

yowl wrote:

Thanks for the reply. I agree its unclear, a replacement could be argued to be both a change and an add. Not sure where the discussions about OldIndex comes from, in all my tests with BindingList, BindingSource, EntityView2 it has always been -1 as described in the docs and I wouldn't suggest you set it to anything else.

Yes I can probably change the code, I can't see why removing the old entity, then adding the new entity will be a problem. I can also use reflection to fire the event again with ItemChanged as that works too.

That would be great simple_smile

You obviously have to decide whether to change your implementation or not. Thats a difficult decision. You have implemented IBindingList with ItemAdded in this scenario, something that is not clear you should do from the documentation, but then again some people may be relying on it. (I would argue they are relying on undocumented behaviour :-)).

yes, it's a bit of a grey area what to do. We chose long ago to use the ItemAdded as it would trigger the view to act upon the newly added entity and in fact, it's adding an entity to the collection, however it's also not really that solid due to the Listchanged drawbacks.

I'm very reluctant to change this, or even change a bit about this, as it could have a lot of consequences and it has a workaround, though I agree there is room for debate whether the current implementation is the ideal one, but at the same time: changing this has consequences which could lead to a break in a code and therefore we won't do that during a release.

In the exhaustive compatibility tests I've done with grids (heavy sarcasm here), I think LLBLGen would be better if it was changed. How about a switch for version 3 ;-) ?

That sounds reasonable, we'll change this to Removed + Added in v3, so the behavior is more consistent with what's to be expected and as it's a new version (and lots of other things will probably break as well) people have to be prepared.

Frans Bouma | Lead developer LLBLGen Pro
simmotech
User
Posts: 1024
Joined: 01-Feb-2006
# Posted on: 30-Jun-2009 20:41:08   

Sorry, but I believe that firing Removed and then Added events is the worst solution of all. A bound grid will look stupid as a row will disappear and then reappear, the focused row will end up in a different place if it happened to be the one being updated, and it will absolutely kill performance too. frowning frowning

A Replacement is not the same as a Remove followed by an Add. If it were then ListChangedType would not need an ItemMoved member, it could simple fire ItemDeleted then ItemAdded. This is an important distinction.

I agree with you that the events fired from a View do not directly mirror those raised from the List (I mean EntityCollection2 here of course but List is shorter and more accurate stuck_out_tongue_winking_eye ) So an ItemChanged raised from the List could result in the View firing a ListChanged with any of the following:- ItemAdded - Where the change means the entity now matches the Filter ItemDeleted - Where the change means the entity now doesn't match the filter ItemMove - Where the change means the entity is now in a different order according to the SortOrder (actually I don't think the View supports this - sends Reset instead) ItemChanged - One or more properties within the entity have changed OR the item has been replaced.

One one level, sure its ambiguous what ItemChanged means: does it mean that the entity contents have changed or that the entity itself has been replaced. But remember that this event doesn't ever refer to a target object only index(es) within a list.

On the other hand, it could be argued that it doesn't matter which type of change caused the event, only that the event did occur: the message says that "the object at index <x> has changed in some way - you may want to reread it and take some action".

I understand that ItemChanged might be a better choice in this case, however it gives a problem due to the ambiguistic behavior: the observer of the view has to perform an instance compare to check whether the object at the given index is indeed the same or it has changed. Is this always done? I have no idea, really

I think my point is that View is not responsible for what the _observer _of the view decides to do with an ambiguous ItemChanged or guessing whether it retained a reference to the original object. It is up to the observer to decide what do but the View has to give the observer a chance to do something- simply hiding the event altogether for a Replacement cannot be correct.

I think that treatment of the two List ItemChanged sources (property changed/item replaced) should be the same. I appreciate you need for backwards compatibility/following standard and the need to not break existing code but a) I doubt that any LLCoolJ user is using the indexer setter on List and listening to ListChanged otherwise they would have queried why it was firing ItemAdded! b) If they were listening to ListChanged on the View, they would have noticed that the indexer setter didn't fire an event for a replacement. c) If they were using the indexer setter on a Collection/View bound to a grid they would have noticed that it is broken anyway.

So I doubt whether anyone would have breaking code, so considering backwards compatibility/following standards with regard to what the EntityCollection/EntityView combination is doing is interesting:

It has some similarities with DataTable/DataView in that it uses a DefaultView by implement IListSource and has Filter/Sorter etc. However, a DataTable doesn't have a similar concept to an Entity, only a set of values to match the columns hence there is no 'row setter' (ie this[] { set } ) function as such and you just replace the values (ie equivalent to property changed). So nothing to be backwards compatible with here.

BindingList<T> is the nearest thing to EntityCollection. It listens to an event on each of its items to determine when something has changed and raise a ListChanged event when it does. As I mentioned, when the change is due to a single PropertyChanged, it uses a 'new' (not sure when this was introduced) ctor on ListChangedEvent to pass the PropertyDescriptor which also sets OldIndex==NewIndex. Undocumented yes, but it is a change to the original v1 behaviour and noone has a problem with it because they assumed that OldIndex was effectively not used (they still can of course or they can check whether the OldIndex==NewIndex and remove the ambiguity for sources that support this). An indexer setter that fires events is 'new' with Bindinglist<T> and so really is the only source AFAIK to a standard, documented or otherwise. It is quite simple, It fires ItemChanged with just the NewIndex set.

At the moment, grid binding is broken with regard to replacing an entity. Dev Express have also come across this ItemChanged ambiguity issue and their solution is to simply assume that ListChanged means PropertyChanged since to assume otherwise would kill performance if they automatically updated the child views. There are a number of articles on their site describing how to manually refresh a Child View in these circumstances. However, since the EntityView is actively stopping the event from even getting to the grid, we have no chance of implementing their workaround.

If ItemChanged is not going to be added (and I sense somehow it isn't unless the above changes your mind smile ), then I would rather no change at all for v2.6 than the ItemAdded/ItemRemoved proposal. Now I know what the problem is, I will just manually fire ItemChanged wherever I replace an entity.

Cheers Simon

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39859
Joined: 17-Aug-2003
# Posted on: 01-Jul-2009 09:57:32   

(edit) for the people who don't know me: I might sound a little too stubborn below. That's not my intention wink . I really don't want to implement delete/add, and I think we'll go for the ItemChanged workaround in v3. I just want to keep things clean and '.NET docs compliant' as we have little other choice considering the fact that users of grids not implementing undocumented workarounds, or only for bindinglist<T> (with hardcoded checks, it won't be the first time) also need to have a working class.

simmotech wrote:

Sorry, but I believe that firing Removed and then Added events is the worst solution of all. A bound grid will look stupid as a row will disappear and then reappear, the focused row will end up in a different place if it happened to be the one being updated, and it will absolutely kill performance too. frowning frowning A Replacement is not the same as a Remove followed by an Add. If it were then ListChangedType would not need an ItemMoved member, it could simple fire ItemDeleted then ItemAdded. This is an important distinction.

I looked again at algorithmia's sourcecode and I didn't look good enough the first time. flushed In the setitem call chain I raise ListChanged there. (with -1). I also checked whether I even use this feature anywhere with databinding at the moment, which isn't the case.

Anyway, I was only thinking about a solution for this or better: a workaround. The problem mainly is that the ListChanged event is flawed and whatever is done, it depends on 3rd party controls whether the workaround will work. History tells us, that that's not the case.

Delete/Add to me sounds like a good combo for replace, simply because the original item gets removed (delete) and a new one is added at that spot (add). Move isn't the same, if you move an item up in the list, you need ItemMoved so the grid knows that.

I do agree with you that delete/add gives problems as well, like the focussed row will go haywire and it might not be aesthetically pleasant for the eye. wink

In general, we really don't want to implement undocumented databinding code for the simple fact that we have to work with all the .NET controls out there and we can't run the risk of our code not working with one of them. It's already a royal pain to make these control vendors obey the standards as they are (how vague they might be) like ITypedList, left alone this.

Delete/Add will work with all grids, but has problems. ListChanged with the index stuff might work with devexpress' grid but not sure which other grid supports this. So there's no real solution, only flawed crappy solutions no-one really wants as both have problems.

As I'm not going to take the blame for this problem, I'll implement one and move on. If one person is happy with the solution I'm sure another one will hate my guts because I didn't pick the other one and vice versa. However it's not our fault: they should target their energy to the company who created the problem: Microsoft and their winforms team. The sad thing is: no-one listens over there... disappointed

I agree with you that the events fired from a View do not directly mirror those raised from the List (I mean EntityCollection2 here of course but List is shorter and more accurate stuck_out_tongue_winking_eye ) So an ItemChanged raised from the List could result in the View firing a ListChanged with any of the following:- ItemAdded - Where the change means the entity now matches the Filter ItemDeleted - Where the change means the entity now doesn't match the filter ItemMove - Where the change means the entity is now in a different order according to the SortOrder (actually I don't think the View supports this - sends Reset instead)

Reset is the one to raise here as the bound control has to repaint itself.

ItemChanged - One or more properties within the entity have changed OR the item has been replaced. One one level, sure its ambiguous what ItemChanged means: does it mean that the entity contents have changed or that the entity itself has been replaced. But remember that this event doesn't ever refer to a target object only index(es) within a list.

On the other hand, it could be argued that it doesn't matter which type of change caused the event, only that the event did occur: the message says that "the object at index <x> has changed in some way - you may want to reread it and take some action".

It is ambiguistic, look at the docs. It can't express what a replace means. If I have an object O at position 3 and I change a property, I get an ItemChanged. If I replace O with O2 at position 3 I also get an item change. However if I see that as 'oh, some property of O has changed' I'm in trouble, I first have to realize that I have to check whether position 3 still contains O and not some other object. The thing is: the docs clearly state that ItemChanged is to be used for property changes on the object at position NewIndex (also a stupid name, but whatever wink ) so I can't do anything else but to follow the docs.

I understand that ItemChanged might be a better choice in this case, however it gives a problem due to the ambiguistic behavior: the observer of the view has to perform an instance compare to check whether the object at the given index is indeed the same or it has changed. Is this always done? I have no idea, really

I think my point is that View is not responsible for what the _observer _of the view decides to do with an ambiguous ItemChanged or guessing whether it retained a reference to the original object. It is up to the observer to decide what do but the View has to give the observer a chance to do something- simply hiding the event altogether for a Replacement cannot be correct.

But ItemChanged isn't to be used for replacements, it's undefined what to use for replacements as the docs leave that open. ItemChanged is to be used for changes in the item at position NewIndex. A replacement isn't that. The one they should have added was ItemReplaced, with NewIndex the index which item was replaced. Perhaps they thought that those 4 bytes were wasted, but they made more mistakes with the event so no surprise there.

I implemented what's in the docs and had to make a decision what to do with replacements. I chose ItemAdded as an item was added. That some grids don't work with that... it's a true bummer, but what can I do? In the docs there's no solution for this and there never will be.

I think that treatment of the two List ItemChanged sources (property changed/item replaced) should be the same. I appreciate you need for backwards compatibility/following standard and the need to not break existing code but a) I doubt that any LLCoolJ user is using the indexer setter on List and listening to ListChanged otherwise they would have queried why it was firing ItemAdded! b) If they were listening to ListChanged on the View, they would have noticed that the indexer setter didn't fire an event for a replacement. c) If they were using the indexer setter on a Collection/View bound to a grid they would have noticed that it is broken anyway. So I doubt whether anyone would have breaking code, so considering backwards compatibility/following standards with regard to what the EntityCollection/EntityView combination is doing is interesting:

doubt is not enough, one has to know nothing breaks. History has learned that either you've to know nothing breaks or don't make the change. The ItemDeleted event is the problem here and if we chose that approach it will break, IF we would make the change in v2.6. As we wont change anything in 2.6 unless it's a bugfix (and non-breaking) anymore, it's a bit of a moot point.

BindingList<T> is the nearest thing to EntityCollection. It listens to an event on each of its items to determine when something has changed and raise a ListChanged event when it does. As I mentioned, when the change is due to a single PropertyChanged, it uses a 'new' (not sure when this was introduced) ctor on ListChangedEvent to pass the PropertyDescriptor which also sets OldIndex==NewIndex. Undocumented yes, but it is a change to the original v1 behaviour and noone has a problem with it because they assumed that OldIndex was effectively not used (they still can of course or they can check whether the OldIndex==NewIndex and remove the ambiguity for sources that support this). An indexer setter that fires events is 'new' with Bindinglist<T> and so really is the only source AFAIK to a standard, documented or otherwise. It is quite simple, It fires ItemChanged with just the NewIndex set.

How odd it might sound, but undocumented == not interesting, for the simple fact that undocumented behavior can 1) change and 2) is likely not implemented the SAME way in all 3rd party controls. There are more undocumented databinding related 'features' (like binding a dataview to a datagridview directly gives different behavior in some edge cases than binding a datatable (which is bound by a view) which suggest hard-coded paths etc. ). In fact, what you describe above is a BREAK with the documented behavior: a property change results in an event argument with BOTH indexes set, not just newindex, as the argument with just newindex set means something ELSE: not a property change but a new item was placed at that spot: this breaks the docs, as the docs suggest otherwise.

I appreciate your time in this and we might choose for this undocumented workaround but I really am not going to dissassemble bindinglist to see what it does in these kind of situations: there is documentation and if that leaves holes open, I have to make a choice, which is NEVER ideal and will never please all.

If ItemChanged is not going to be added (and I sense somehow it isn't unless the above changes your mind smile ), then I would rather no change at all for v2.6 than the ItemAdded/ItemRemoved proposal. Now I know what the problem is, I will just manually fire ItemChanged wherever I replace an entity.

v2.6 won't be changed anymore. We've to decide what to do with v3.0's runtime in this case: implement delete/add which sucks, or implement itemchanged which will likely not work with some grids but we don't know (however currently it also doesn't work, so no change there but if we say "fixed!" and it's not, we have a problem. )

After reading your post I'm leaning towards Itemchanged, as the delete/add has drawbacks like the focussed row which is really annoying and IMHO more annoying than that some grids don't support this workaround (as in general grids all suck in one way or the other, including xrtagrid). However we won't look at this again till v3.0's runtime libs are on the bench to be ported to v3's features.

Frans Bouma | Lead developer LLBLGen Pro
simmotech
User
Posts: 1024
Joined: 01-Feb-2006
# Posted on: 01-Jul-2009 12:39:21   

I don't mind stubborn. Stubborn is good smile And I won't be offended if you won't be.

Ok, so 2.6 won't be changed - good, thats sorted.

Workaround for anyone reading this thread who doesn't like reflection: Add this to a public partial class EntityCollection<TEntity> where TEntity : EntityBase2, IEntity2 - both ways are then available

    public void ReplaceItem(int index, TEntity replacementEntity)
    {
        this[index] = replacementEntity;
        OnListChanged(index, ListChangedType.ItemChanged);
    }

Reset is the one to raise here as the bound control has to repaint itself.

This will of course work, but you're making an assumption here that something is bound that requires repainting and it also doesn't fully support ItemMove. A listener should be prepared to deal with an ItemMove and either treat it as a Reset (as a catch all) or optimize any repainting (or reindexing or whatever it happens to do) by adjusting itself only for those items between OldIndex and NewIndex. In fact, if it was a grid listener and the index range was not currently being displayed it could treat it as a NOP. Alteratively, it might just need to reread a subset of the list.

It is ambiguistic, look at the docs.

and

The thing is: the docs clearly state that ItemChanged is to be used for property changes on the object at position NewIndex

and

But ItemChanged isn't to be used for replacements, it's undefined what to use for replacements as the docs leave that open. ItemChanged is to be used for changes in the item at position NewIndex. A replacement isn't that.

For the docs, I'm looking at, under ListChangedType Enumeration, it states that

ItemChanged An item changed in the list. NewIndex contains the index of the item that was changed. (my emphasis). It doesn't mention anything about the contents/property of the entity changing at all. To me, all this says is that something changed in the list at this index - nothing more than that - and so a Replace meets this definition just as well as a property changed. Do you know of another place in the documentation that contradicts this? Theres not some confusion over the ItemChanged event of CurrencyManager is there?

Some listeners might maintain a reference to the item at a notional 'current' position, CurrencyManager for example, but even those that do can simple compare their copy with the current copy to differentiate if necessary. Listeners that don't keep a local reference to any item and just work on indices, EntityView for example, shouldn't care whether the item at the index has changed itself or one of its properties - their processing should be the same. Further, any differentiation they make (unless a PropertyDescriptor is present on the EventArgs) is inferred rather than documented.

BindingList<T>: (Reflector it - seriously its a very short, simple class) Well I agree that the documentation may not specifically document how it handles the two types of ItemChanged but its actual implementation and the 2 ctors on ListChangedEventArgs that support PropertyDescriptors are now in the public domain and so they can't be changed without breaking backwards compatibility - even MS won't do this.

With regard to v3, I'm glad your looking at the ItemChanged route. I obviously haven't seen the code for v3 but if it is similar to v2.6, then the only change would be that

HandleRelatedCollectionItemChanged

gets called instead of

HandleRelatedCollectionItemAdded

so the processing for both types of Change is the same. I can't see anything in EntityView that relies on an entity being the same one at a given index.

Cheers Simon

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39859
Joined: 17-Aug-2003
# Posted on: 02-Jul-2009 09:20:20   

simmotech wrote:

Reset is the one to raise here as the bound control has to repaint itself.

This will of course work, but you're making an assumption here that something is bound that requires repainting and it also doesn't fully support ItemMove. A listener should be prepared to deal with an ItemMove and either treat it as a Reset (as a catch all) or optimize any repainting (or reindexing or whatever it happens to do) by adjusting itself only for those items between OldIndex and NewIndex. In fact, if it was a grid listener and the index range was not currently being displayed it could treat it as a NOP. Alteratively, it might just need to reread a subset of the list.

The problem is that for every row in the new, sorted situation, an event has to be raised, which is truly not what you want as it will choke event handlers. the main cause of slowness in a UI is due to event handlers even on fast machines. So 1 event vs. say 100, is really a no brainer, also because the grid has to re-render a part just once, which can be faster as well. I still think a reset is best in the case of sort (and no, I'm not going to change that wink )

It is ambiguistic, look at the docs.

and

The thing is: the docs clearly state that ItemChanged is to be used for property changes on the object at position NewIndex

and

But ItemChanged isn't to be used for replacements, it's undefined what to use for replacements as the docs leave that open. ItemChanged is to be used for changes in the item at position NewIndex. A replacement isn't that.

For the docs, I'm looking at, under ListChangedType Enumeration, it states that ItemChanged An item changed in the list. NewIndex contains the index of the item that was changed. (my emphasis). It doesn't mention anything about the contents/property of the entity changing at all. To me, all this says is that something changed in the list at this index - nothing more than that - and so a Replace meets this definition just as well as a property changed. Do you know of another place in the documentation that contradicts this? Theres not some confusion over the ItemChanged event of CurrencyManager is there?

I'm not a native english speaker, you are, so it might be due to language barriers, but ... if something says "An item changed in the list. NewIndex contains the index of the item that was changed", I don't read in that that the item was replaced, but that the item itself got its contents changed. How else would you describe in english the situation that an item in the list got changed? If you want to describe a replacement, I don't think one would use the phrase as used in the docs. But you can read multple things in that line, I agree.

BindingList<T>: (Reflector it - seriously its a very short, simple class) Well I agree that the documentation may not specifically document how it handles the two types of ItemChanged but its actual implementation and the 2 ctors on ListChangedEventArgs that support PropertyDescriptors are now in the public domain and so they can't be changed without breaking backwards compatibility - even MS won't do this.

True, but implementing the behavior of bindinglist doesn't make it work. Control vendors sadly have a trackrecord of checking the type of the bound object and act accordingly.

With regard to v3, I'm glad your looking at the ItemChanged route. I obviously haven't seen the code for v3 but if it is similar to v2.6, then the only change would be that

HandleRelatedCollectionItemChanged

gets called instead of

HandleRelatedCollectionItemAdded

so the processing for both types of Change is the same. I can't see anything in EntityView that relies on an entity being the same one at a given index.

V3.0's runtime lib will largely be the same as v2.6 with some changes for new features like value types (DDD) and mixed inheritance hierarchy types. I was more thinking in raising the ItemChanged event from the collection instead of changing the route of the event in the view. After all, the source of the problem is IMHO in the collection's indexer setter where ItemAdded is raised, which doesn't have the effect it should have. wink

Frans Bouma | Lead developer LLBLGen Pro
simmotech
User
Posts: 1024
Joined: 01-Feb-2006
# Posted on: 02-Jul-2009 10:18:08   

The problem is that for every row in the new, sorted situation, an event has to be raised, which is truly not what you want as it will choke event handlers. the main cause of slowness in a UI is due to event handlers even on fast machines. So 1 event vs. say 100, is really a no brainer, also because the grid has to re-render a part just once, which can be faster as well. I still think a reset is best in the case of sort (and no, I'm not going to change that Wink)

No, there is still only one event to be raised - an ItemMoved. I still think you are making assumptions about who the listener(s) are and how they work. The ListChanged event is completely data and GUI agnostic - all it does is describe to listeners in the simplest possible terms what has changed in the list - doesn't need to do more than that. It is up to the listener to deal with the information in whatever way it deems fit. If you send a Reset, then a Grid (as an example listener) has no chance to optimize how it displays the change. It has to reread every item in the entire list as that is what Reset means. If you send an ItemMoved, then it might do that anyway but it might not depending on how clever it is. I've no problem with you sending Reset but it isn't optimal. You are telling the listener that 'Much of the list has changed. Any listening controls should refresh all their data from the list."

I'm not a native english speaker, you are, so it might be due to language barriers, but ... if something says "An item changed in the list. NewIndex contains the index of the item that was changed", I don't read in that that the item was replaced, but that the item itself got its contents changed. How else would you describe in english the situation that an item in the list got changed? If you want to describe a replacement, I don't think one would use the phrase as used in the docs. But you can read multple things in that line, I agree.

Similar to the above, I believe the ListChanged event is data agnostic. If you had a BindingList<int>, then setting a new int in the list is a Replace but its 'contents' haven't because it doesn't have any properties of its own.

V3.0's runtime lib will largely be the same as v2.6 with some changes for new features like value types (DDD) and mixed inheritance hierarchy types. I was more thinking in raising the ItemChanged event from the collection instead of changing the route of the event in the view. After all, the source of the problem is IMHO in the collection's indexer setter where ItemAdded is raised, which doesn't have the effect it should have. Wink

So was I. What I meant was that once the ItemChanged (I assume you mean ListChanged with ListChangedType.ItemChanged in the args) is fired, the View will automatically go through the other route I mentioned and, I believe, just work.

Cheers Simon

PS How are value types (DDD) going to be used in LLCoolJ? Or is it still a secret at the moment?

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39859
Joined: 17-Aug-2003
# Posted on: 02-Jul-2009 14:16:58   

simmotech wrote:

The problem is that for every row in the new, sorted situation, an event has to be raised, which is truly not what you want as it will choke event handlers. the main cause of slowness in a UI is due to event handlers even on fast machines. So 1 event vs. say 100, is really a no brainer, also because the grid has to re-render a part just once, which can be faster as well. I still think a reset is best in the case of sort (and no, I'm not going to change that Wink)

No, there is still only one event to be raised - an ItemMoved.

if I sort a view on a property, every row could be placed at a new index, which means every row has been moved to a new location, which means for every row an ItemMoved has to be raised (as it is per item). Therefore reset is better.

I still think you are making assumptions about who the listener(s) are and how they work. The ListChanged event is completely data and GUI agnostic - all it does is describe to listeners in the simplest possible terms what has changed in the list - doesn't need to do more than that. It is up to the listener to deal with the information in whatever way it deems fit. If you send a Reset, then a Grid (as an example listener) has no chance to optimize how it displays the change. It has to reread every item in the entire list as that is what Reset means. If you send an ItemMoved, then it might do that anyway but it might not depending on how clever it is. I've no problem with you sending Reset but it isn't optimal. You are telling the listener that 'Much of the list has changed. Any listening controls should refresh all their data from the list."

I don't follow. If I have 100 items in the view, and I sort it on a different property so all rows are at a different location, I can't use 1 ItemMoved, I have to use a Reset. That's why the reset is issued. (similar to what DataView does IIRC)

I'm not a native english speaker, you are, so it might be due to language barriers, but ... if something says "An item changed in the list. NewIndex contains the index of the item that was changed", I don't read in that that the item was replaced, but that the item itself got its contents changed. How else would you describe in english the situation that an item in the list got changed? If you want to describe a replacement, I don't think one would use the phrase as used in the docs. But you can read multple things in that line, I agree.

Similar to the above, I believe the ListChanged event is data agnostic. If you had a BindingList<int>, then setting a new int in the list is a Replace but its 'contents' haven't because it doesn't have any properties of its own.

For the int it doesn't matter indeed, but for an object it really does matter, it's something else. Maybe you don't want to make that distinction but I do. The docs aren't clear about that, at least not for me, I find them ambiguistic (and BindingList proves that as it uses OldIndex to signal replace vs. property change, which isn't documented)

PS How are value types (DDD) going to be used in LLCoolJ? Or is it still a secret at the moment?

More or less as you'd expect: CustomerEntity.VisitingAddress returns an Address instance which is a value type which has the fields Zipcode, StreetName, HouseNr, City. You then could do: myCustomer.BillingAddress = myCustomer.VisitingAddress and the billing address fields in the db get the same values. So valuetypes aren't entities though they do contain fields. You can't save them individually, they only live within an entity instance.

One of the problems we haven't solved yet is how to use them in queries using our own query api. Obviously Customer.VisitingAddress.City is a different field than Customer.BillingAddress.City, yet it's the same 'field' (Address.City). In the designer we solved this through 'paths' (so a full path from entity to end field), perhaps we'll add that to the runtime as well. There are other solutions, we're not working on the runtime yet (code generator first has to work wink ).

Value types are generated as partial classes as well, and we want to add validation support to them as well, so you can for example create a special field type with its own validation in a partial class of the generated code.

Frans Bouma | Lead developer LLBLGen Pro
1  /  2