Move an entity within a collection

Posts   
 
    
simmotech
User
Posts: 1024
Joined: 01-Feb-2006
# Posted on: 25-Feb-2013 14:59:34   

Is there a way I have missed to be able to simply move an entity within a collection?

I want to do the equivalent of this...

OwnershipEntities.Remove(existingItem);
OwnershipEntities.Insert(clientInsertIndex++, existingItem);

... where existingItem has been found within OwnershipEntities.

(I can't use this code directly because my Auditor will put existingItem into the deleted entities collection as soon as it is removed)

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39910
Joined: 17-Aug-2003
# Posted on: 25-Feb-2013 15:52:30   

Currently, no. Could you describe the use-case for this? One way to work around this is to add a simple int property to the entity in the collection and sort on that property in a bound control (I assume this is used in an UI where the user specifies order by clicking up/down buttons) You can extend this to persist this ordering field in the DB as well, so you don't rely on the order in which the DB stores the rows / inserts.

The idea is that the entities in a collection is unordered, the views on it are ordered in a given way. As you can have multiple views on the same collection with different sorters, 'order' is not a concept in the collection although they're ordered in an enumerable way, but that's not based on a rule.

Frans Bouma | Lead developer LLBLGen Pro
simmotech
User
Posts: 1024
Joined: 01-Feb-2006
# Posted on: 25-Feb-2013 17:56:55   

The idea is that the entities in a collection is unordered, the views on it are ordered in a given way. As you can have multiple views on the same collection with different sorters, 'order' is not a concept in the collection although they're ordered in an enumerable way, but that's not based on a rule.

I knew you were going to say that smile And I normally agree but there are issues doing it that way also.

Here is the use case: I have an AssetEntity which is 'owned' by a number of legal bodies'. So Asset has a 1:m with AssetToOwner. There is no particular limit to the number of Ownerships.

Some of these Owners may be 'Clients' - say Client1 and Client2. They are treated specially in that they are always listed first and Client1 always comes before Client2. Each Asset has a custom property called OwnerName which is a simple string summary and looks like "<Client1FirstName>, <Client2FirstName> plus xxx others". Now I can use Linq to pick out those clients and format the string. It would be easier and quicker if they were both at the top of the list but it works ok. All the ownerships are displayed in a grid at the same time and I definitely need them displayed at the top there. Moving those two to the top of the collection makes it easy to display in a grid but gives the problems I described above if I Remove then Insert.

So, using a View to achieve the same thing. Well the FieldName and value is sort of known but I couldn't see a way of providing a custom comparer to the EntityView2. So I cheated and came up with this:-

        public class MyEntityProperty: EntityProperty, IEntityFieldCoreInterpret
        {
            readonly Dictionary<int, int> clientIDSubstitutions = new Dictionary<int, int>();

            public MyEntityProperty(IEnumerable<ClientEntity> clients): base(string.Empty)
            {
                var clientSubstituteID = int.MinValue;

                foreach (var client in clients)
                {
                    clientIDSubstitutions[client.ID] = clientSubstituteID++;
                }
            }

            public object GetValue(IEntityCore entity)
            {
                var legalBodyID = ((ILegalBodyOwnershipEntity) entity).LegalBodyID;
                int result;

                if (clientIDSubstitutions.TryGetValue(legalBodyID, out result))
                {
                    return result;
                }

                return legalBodyID;
            }
        }

This seems to work but I have another problem now in that when I add a large number of non-client users (6,500) using AddRange, whereas the Remove/InsertAtZero method took 5 seconds at the most, it is now taking forever. And I can see why - AddRange is just calling Add for each item (frowning )! And when it gets to PerformAdd, it is calling OnListChanged (frowning frowning ), and since there is now a View listening, it is doing a resort, after single Add (frowning frowning frowning ).

What should happen is that AddRange should just fire a single OnListChanged(Reset) after adding all the items internally.

I had already added special BeginUpdate()/EndUpdate() to suspend myriad updates received by the entity owner of the collection but I might need to do the same for the collection itself so that the view doesn't see them.

All for the want of a MoveEntityToADifferentIndex call.

simmotech
User
Posts: 1024
Joined: 01-Feb-2006
# Posted on: 25-Feb-2013 18:49:05   

Just tried hiding all the intermediate OnListChanged events with this code....

        int lockUpdate;

        public void BeginUpdate()
        {
            lockUpdate++;
        }

        public bool IsUpdateLocked
        {
            get { return lockUpdate > 0; }
        }

        public void EndUpdate()
        {
            if (--lockUpdate == 0)
            {
                OnListChanged(-1, ListChangedType.Reset);
            }
        }

        protected override void OnListChanged(int index, ListChangedType typeOfChange)
        {
            if (IsUpdateLocked) return;
            ... etc....

Brilliant! 6,500 items added and shown in the grid instantaneously. They have the clients as the first two items and the summary string is correct.

But just tried a save, via a unit of work, and found that OnEntityInListOnEntityContentsChanged is being called and since the View is a listener, it is still doing a resort for each of 6,500 ItemChanged messages.

Don't really know where to go from here. It was easy to wrap the AddRange with a BeginUpdate()/EndUpdate() but the UOW save is a very large entity graph in a different part of the app - no easy way to manually suspend updates.

Walaa avatar
Walaa
Support Team
Posts: 14995
Joined: 21-Aug-2005
# Posted on: 25-Feb-2013 22:38:40   

Why don't you unbound the view from the collection before doing the updates, then bind it again when finished.

simmotech
User
Posts: 1024
Joined: 01-Feb-2006
# Posted on: 26-Feb-2013 07:47:00   

The code that updates the collection knows nothing about any View(s) listening to the collection.

It is really the collections responsibility to be able to make multiple changes and fire a Reset to make it 'atomic'. But Lord knows how the update-after-Save can be got around.

simmotech
User
Posts: 1024
Joined: 01-Feb-2006
# Posted on: 26-Feb-2013 10:12:27   

Additional EntityCollection<T> methods:

This code can be used to do a direct Move

        public int MoveItemTo(int newIndex, TEntity target)
        {
            var list = ReflectionHelper.GetNonPublicFieldValue<List<TEntity>>(this, "_contents");

            var sourceIndex = list.IndexOf(target);

            if (sourceIndex != -1)
            {
                list.RemoveAt(sourceIndex);
                list.Insert(newIndex, target);

                OnListChanged(-1, ListChangedType.Reset);
            }

            return sourceIndex;
        }

and this code is an alternative way of suspending ListChanged events:

        public void PerformMultiAction(Action action)
        {
            var changedEventsInternal = SuppressListChangedEventsInternal;
            SuppressListChangedEventsInternal = true;

            try
            {
                action();
            }

            finally
            {
                SuppressListChangedEventsInternal = changedEventsInternal;
                OnListChanged(0, ListChangedType.Reset);
            }
        }

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39910
Joined: 17-Aug-2003
# Posted on: 27-Feb-2013 15:39:54   

About AddRange() it indeed raises the events every time. This is by design. The problem with a 'Reset' is that an active row in the grid is lost. So if we raise a Reset, people who rely on that will be up in arms. If we don't, people like you raise performance concerns. Sadly, we can't do both.

To avoid a lot of view changes due to a sorter/filter which is applied all the time, set DataChangeAction on the view to PostCollectionChangeAction.NoAction. Then after the addRange call, set it back to the value you want and call Refresh on the view.

About your use case In all honesty, if you want ordering like that, it's IMHO best to specify an ordering in the form of a value. Take for example this forum. The order of the forums in a section is not random, it's determined by a sortvalue, a simple int, specified with the forum.

You can accomplish this outside the DB as well, as it doesn't matter where the property lives. the view sorter simply sorts the values referred to by the sortclauses set. So in theory you could also use an ISortClause expression which returns a field object which is returning interpret values which guarantee the order you want. However a custom public int SortValue { get; set;} is easier to add/work with.

About move The problem with move is this: what is 'index' ? If I move item X on index 10 to index 12, is '12', what do I do? -> move item from 10 to tmp, move item from 11 to 10, item from 12 to 11, item from tmp to 12? Or do I leave slot 10 open? It's a list, but not a queue/sequence. There is a difference:

var list = new List<int>(100);
list[10] = 4;

if it's a sequence, this isn't possible, as there's just 1 value in the list. So what does 'move' really mean? If I use the methods available on a List, I get effectively what I have specified above: I first remove the item from index 10, all items move forward. Then I insert it again at 12, all items from 12 and up move 1 slot up.

But if I fall back to index manipulation, this goes out the window: nothing moves if I simply set an item at index I in a list. So there are two things here.

The collection can have a view, it doesn't know that, nor cares: it simply signals these bound objects that some items moved positions if you call RemoveAt and Insert to move an item from one index to another. This is necessary because the view works with indexes, it doesn't store references to entities, it stores indexes. So if a filter is applied, and the entities at 9, 12 and 22 are in the view, and you move an entity from 10 to 14, what happens? If I use array access (the this[] indexer) nothing changes, but that's not correct, item '12' should move to '11', so the view should update its indexes, namely that item '12' should now be '11'.

IMHO also the reason why IList and friends doesn't have a Move method, simply because it's equal to RemoveAt() and Insert. That both methods have side effects is indeed something to take into account, but not a reason for the collection to have all kinds of logic which is actually related to looking at the data, not the data itself. Ordering of rows in a collection is related to looking at the data, nothing else.

So I'd simple use the property for sorting, it's the least amount of coding and works every time.

Frans Bouma | Lead developer LLBLGen Pro
simmotech
User
Posts: 1024
Joined: 01-Feb-2006
# Posted on: 27-Feb-2013 17:46:40   

The code I mentioned works well (with an additional check for sourceIndex != newIndex). I wasn't expecting it to be added to your code but I included it there for anyone who has a similar problem.

The problem with a 'Reset' is that an active row in the grid is lost

Only with a crap grid smile - DevExpress will keep the same row. But Reset has a specific meaning - it means more than a simple change has occured.

PostCollectionChangeAction.NoAction won't work in this case because the code updating the EntityCollection knows nothing about any Views that are listening.

It could be argued that an AddRange method on any list is pointless since the same could be achieved by repeatedly calling Add(). AddRange is there to optimize >1 Add() and Reset is there specifically to say "much of the list has changed". Haven't we had this conversion before smile

My use case: This is a bit unusual in that only a maximum of two entities need to be moved to the 'front of the list'. Using a sorting View was overkill for that. But the problem was not doing the sorting per se, but the number of times it was performed. 6,500 times in this case.

About Move: Actually I think a List is a sequence (with direct access). Using indexers will fail if the index is outside that sequence as you point out but that wasn't what I wanted to do. Just move an existing entity from its current position to another position. No need to leave slots open or anything like that.

The actual correct operation would be to fire a ListChanged.ItemMoved with the sourceIndex and destinationIndex - everything between those is now different and the listener should either work out what or treat it as a Reset event and assume everything changed.

A Move() and a RemoveAt() + Insert() are not equal IMHO (we definitely had this conversion before stuck_out_tongue_winking_eye ). Only the final result is the same: In the former, the Count never changes, in the latter it does, twice. Plus the shuffling around after the Remove and then back again after the Insert - means all items between the two indexes have had their indexes changed twice. The former is or should be an atomic operation but the latter most certainly isn't

Ordering of rows in a collection is related to looking at the data, nothing else

Well, although it is called EntityCollection<T>, it is still actually a List and therefore has ordering of its contents. I agree that, mostly, this distinction doesn't matter - either a grid or an EntityView will arrange the ordering appropriately for display.

So I'd simple use the property for sorting, it's the least amount of coding and works every time. 

But it was completely unusable - several minutes to add some items to a list.

Walaa avatar
Walaa
Support Team
Posts: 14995
Joined: 21-Aug-2005
# Posted on: 27-Feb-2013 18:43:26   

I think you should use a "LinkedList" to hold your entities. Or some structure to cater for the order rather than the entityCollection.

You can have helper functions to copy entities back and forth between the entitycollection and the LinkedList, when you need to fetch entities or save them back to the database.

simmotech
User
Posts: 1024
Joined: 01-Feb-2006
# Posted on: 28-Feb-2013 07:43:49   

Heavy databinding is involved so implementing all of the other interfaces on another collection type would be prohibitive.

As I said, EntityCollection is fine - I just have an issue with the way AddRange was implemented. Too late to change now even if Frans evntually agrees with my interpretation of it (which is doubtful smile ). But those two methods I showed above get around the issue, I can add 6000+ entities virtually instantly without regard to anything listening to the EntityCollection and if a View happens to be attached, it will only need to sort them just the once.

PS Have you reduced the size of some of the text on these forum pages? The "You are here" line seems to be about 6pt text - or am I just getting old and my eyesight is going.

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39910
Joined: 17-Aug-2003
# Posted on: 28-Feb-2013 11:37:02   

simmotech wrote:

Heavy databinding is involved so implementing all of the other interfaces on another collection type would be prohibitive.

As I said, EntityCollection is fine - I just have an issue with the way AddRange was implemented. Too late to change now even if Frans evntually agrees with my interpretation of it (which is doubtful smile ). But those two methods I showed above get around the issue, I can add 6000+ entities virtually instantly without regard to anything listening to the EntityCollection and if a View happens to be attached, it will only need to sort them just the once.

With the setting on the view to not update itself if the collection changes will accomplish the same actually. The event handler will raise, but the view will not do anything.

PS Have you reduced the size of some of the text on these forum pages? The "You are here" line seems to be about 6pt text - or am I just getting old and my eyesight is going.

No, nothing has changed. You are here is small text btw. Please check whether you accidentally zoomed on this website or not, by resetting it in firefox with cntrl-0

Frans Bouma | Lead developer LLBLGen Pro
simmotech
User
Posts: 1024
Joined: 01-Feb-2006
# Posted on: 28-Feb-2013 12:39:15   

With the setting on the view to not update itself if the collection changes will accomplish the same actually. The event handler will raise, but the view will not do anything.

Sure, but separation-of-concerns means that the code adding to the collection has no knowledge of any View - that is in an entirely separate part of the app - and the bit using the View will never know that something is about to make some big changes to the underlying collection.

Go on, admit it...If you had to do it all over, you would implement AddRange like I said and told anyone with grid problems to use a for-each to add them one at a time. smile

PS - Thanks for the tip on the browser (Chrome in my case) - I didn't know it remembered zooms on a per-website basis!

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39910
Joined: 17-Aug-2003
# Posted on: 01-Mar-2013 10:49:08   

simmotech wrote:

With the setting on the view to not update itself if the collection changes will accomplish the same actually. The event handler will raise, but the view will not do anything.

Sure, but separation-of-concerns means that the code adding to the collection has no knowledge of any View - that is in an entirely separate part of the app - and the bit using the View will never know that something is about to make some big changes to the underlying collection.

Sure, but in that same logic, the collection doesn't care whether events raised from it cause problems elsewhere.

Go on, admit it...If you had to do it all over, you would implement AddRange like I said and told anyone with grid problems to use a for-each to add them one at a time. smile

No, as there were grid problems in the past we have solved, that's why I knew this could occur. Personally, I don't really care if some grid messes up, they all suck in one way or the other at some point, but what I think doesn't matter (unfortunatelywink ), if some people have problems, the best thing to do is to fix it for them. I do agree that addrange can be more efficient in this regards.

There's a Suppress events property on the collection, it's internal at the moment. Let me think about this, what's the best thing to do: make the change in AddRange to do a reset and file it as a breaking change, or make the Suppress events property protected, so one could use that in a partial class of collection and leave AddRange as-is.

PS - Thanks for the tip on the browser (Chrome in my case) - I didn't know it remembered zooms on a per-website basis!

I searched for ages for a clue why images were distorted on my blog in my browser and no-where else, till I figured out zooming is indeed stored per website. simple_smile

Frans Bouma | Lead developer LLBLGen Pro
simmotech
User
Posts: 1024
Joined: 01-Feb-2006
# Posted on: 01-Mar-2013 13:23:33   

Sure, but in that same logic, the collection doesn't care whether events raised from it cause problems elsewhere.

Exactly, but IMHO it shouldn't care. As long as it provides enough info about what changed, it is up to the downstream listeners to deal with that gracefully.

SuppressListChangedEventsInternal is already on CollectionCore<T> as a protected member. Its also available via ICollectionMaintenance.SurpressListChangedEvents (note the spelling mistake!)

My opinion is that AddRange should be changed. I don't think AddRange is used within the LLBLGen libraries, except one ctor which shouldn't matter since noone can be listening at that point and a redirect from IEntityCollection2. So it would be down to the caller to choose AddRange/Reset or many x Add if they don't want to deal with a Reset.

But If you don't want a breaking change then the PerformMultiAction sample code I listed above can wrap an AddRange call to give the same effect. It is useful in its own right since any actions can be wrapped as long as listeners understand a Reset event.

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39910
Joined: 17-Aug-2003
# Posted on: 04-Mar-2013 17:19:23   

simmotech wrote:

Sure, but in that same logic, the collection doesn't care whether events raised from it cause problems elsewhere.

Exactly, but IMHO it shouldn't care. As long as it provides enough info about what changed, it is up to the downstream listeners to deal with that gracefully.

SuppressListChangedEventsInternal is already on CollectionCore<T> as a protected member. Its also available via ICollectionMaintenance.SurpressListChangedEvents (note the spelling mistake!)

I've corrected the spelling mistake in v4. flushed

My opinion is that AddRange should be changed. I don't think AddRange is used within the LLBLGen libraries, except one ctor which shouldn't matter since noone can be listening at that point and a redirect from IEntityCollection2. So it would be down to the caller to choose AddRange/Reset or many x Add if they don't want to deal with a Reset.

Yeah, I've thought about it some time and indeed, it's best to modify it to suppress the listchanged events and replace it with a Reset if something was added. After all, the AddRange method pre-allocates new space at once for the new items, if required.

So in v4, the AddRange will raise a Reset afterwards, not ItemAdded. All other events are still issued, only the ListChanged is suppressed.

Frans Bouma | Lead developer LLBLGen Pro