List<T> remove item whilst iterating.

[chef 3745]

This has always been the Bain of my coding exsistance. You create a a List then as you deal with items in the list you want to remove them.

 

But, you can not change the collection of items while you are reading through them.

 

Some people have mentioned using a for loop backwards through the list

 

Is that the actual trick to being able to remove items while reading through them?

 

Seems strange to me that that is the answer. Lol.

[ebr 14912]

With an actual List object I don't think the backwards trick will work anyway.

 

The best thing to do is design your logic so that you can remove items outside the iteration.  I don't know exactly what you are trying to do here but, if you must iterate the list to find items to remove, then simply save a reference to those items in your iteration (in another list or array or whatever) and then iterate that list afterwards deleting each of the items you saved.

[chef 3745]

With an actual List object I don't think the backwards trick will work anyway.

 

The best thing to do is design your logic so that you can remove items outside the iteration.  I don't know exactly what you are trying to do here but, if you must iterate the list to find items to remove, then simply save a reference to those items in your iteration (in another list or array or whatever) and then iterate that list afterwards deleting each of the items you saved.

Yes I figured that would be the answer. 

 

After looking into how List<T> actually work in .Net, you can see that MS keeps a "_version" and "version" number of the list. Each time the List changes the list version increments. Then "_version" is compared to "version" on each iteration. If they don't match the compiler is written to throw an error. 

 

 

Creating a "tempList" to compare items to remove will have to do. But, also keeping in mind that you literally have to create a new List<T>.

 

code like:

var tempList = MyOriginalList;

won't work, because anytime you change "tempList" then "MyOrininalList" will actually change as well.

 

it has to be (complicated obviously):

List<int> MyOriginalList = new List<int>(){1,2,3}

List<int> tempList = MyOriginalList.Select(i => new int {i}).ToList();

Or something like that....

 

Mostly I'll just "tempList.Add(item)" while looping.

 

If the item appears in both list remove it from "MyOriginalList"

 

Thank goodness "MyOriginalList" will not contain duplicate entries....

 

I'll figure it out...

 

Many thanks ebr 

Edited July 3, 2017 by chef

[ebr 14912]

What I meant by a second list was to keep a list of just the items you want to remove - not clone the original.  Like so (psuedo code):

List toRemove

foreach item in list {
  if (item should be removed) {
     toRemove.add(item)
  }
}

foreach item in toRemove {
  list.remove(item)
}

But, again, I'd examine what you are actually trying to accomplish to see if you even really need to do this type of iteration.

[mwongjay 64]

Use linq and exclude the items you don't want?

var myOriginalList = new Collection<int> { 1, 2, 3 };

var unwantedItemsList = new Collection<int>();
unwantedItemsList.Add(2);

var originalListMinusUnwanted = myOriginalList.Except(unwantedItemsList);

Or iterate and return only the values you need

var myOriginalList = new Collection<int> { 1, 2, 3 };
var wantedItems = GetWantedItems(myOriginalList);

//...

public static IEnumerable<int> GetWantedItems(IEnumerable<int> originalList) {
    foreach(var originalListItem in originalList)
    {
        // Process list item
        if (originalListItem % 2 == 1)
            yield return originalListItem;
    }
}

@@chef If you post your code and what you're trying to do it will make it easier for another dev to help you out. 

[chef 3745]

Sweet I appreciate the help actually. 

 

My plugin config class:

    public class PluginConfiguration : BasePluginConfiguration
    {
      
        public List<Message> Messages { get; set; }
      
    }

Message class:

    public class Message 
    {
        public string Header { get; set; }
        public string Text { get; set; }
        public string User { get; set; }
        public bool PausePlayback { get; set; }
        public bool Popup { get; set; }
        public string MessageDate { get; set; }
        public List<string> Recipients { get; set; }
        public bool SendOnSessionStart { get; set; }
        public long Timeout { get; set; }
    }

It is the Recipient List in the Message class which must be altered.  

 

When a user receives a message their name must be removed from the Recipient List<string>, then the Message has to be rebuilt and saved back to the List<Message> in the plugin configuration.

 

I wanted to keep the code light, but when dealing with Lists inside of Lists it starts to get a bit complicated.

[chef 3745]

I have come up with this:

        public static Message RemoveRecipient(Message m, string userName)
        {
            var recipientList = new List<string>();

            var messages = Plugin.Instance.Configuration.Messages;

            for (int i = messages.Count - 1; i >= 0; i--)
            {
                if (messages[i].MessageDate == m.MessageDate)
                {
                    recipientList.AddRange(messages[i].Recipients.Where(recipient => recipient != userName));

                    var mes = new Message
                    {
                        Recipients = recipientList,
                        Header = messages[i].Header,
                        MessageDate = messages[i].MessageDate,
                        PausePlayback = messages[i].PausePlayback,
                        Popup = messages[i].Popup,
                        SendOnSessionStart = messages[i].SendOnSessionStart,
                        Text = messages[i].Text
                    };

                    return mes;

                }
            }
            return null;
        }

Edited July 3, 2017 by chef

[chef 3745]

Then I check and recreate the whole list like this i think:

        public static void CheckStatus(Object stateInfo)
        {
            // Stop the timer while we do work
            Plugin.StateTimer.Change(Timeout.Infinite, Timeout.Infinite);
            
            var tempMessageList = new List<Message>();
            foreach (var message in Plugin.Instance.Configuration.Messages)
            {
                foreach (var session in SessionManager.Sessions)
                {
                    if (message.SendOnSessionStart && message.Recipients.Contains(session.UserName))
                    {
                        var ms = new MessageService(SessionManager, UserManager, Logger);
                        ms.Post(message);
                        tempMessageList.Add(RemoveRecipient(message, session.UserName));
                    }
                    else
                    {
                        tempMessageList.Add(message);
                    }
                }
            }

            Plugin.Instance.UpdateConfiguration(new PluginConfiguration()
            {
                Messages = tempMessageList
            });
            
        }

[mwongjay 64]

Message class

public class Message
    {
        private readonly Collection<string> _recipients;

        public Message(
            string header,
            string text,
            string user,
            bool pausePlayback,
            bool popup,
            string messageDate,
            Collection<string> recipients,
            bool sendOnSessionStart,
            long timeout)
        {
            Header = header;
            Text = text;
            User = user;
            PausePlayback = pausePlayback;
            Popup = popup;
            MessageDate = messageDate;
            _recipients = recipients;
            SendOnSessionStart = sendOnSessionStart;
            Timeout = timeout;
        }

        public string Header { get; }
        public string Text { get; }
        public string User { get; }
        public bool PausePlayback { get; }
        public bool Popup { get; }
        public string MessageDate { get; }
        public IEnumerable<string> Recipients => _recipients;
        public bool SendOnSessionStart { get; }
        public long Timeout { get; }

        public void RemoveRecipient(string username)
        {
            _recipients.Remove(username);
        }
    }

CheckStatus - I would recommend changing this to be more concise/clear about its function (i.e. NotifyAndUpdateRemainingRecipients)

// Inject these dependencies
private readonly ISessionManager _sessionManager;
private readonly IMessageService _messageService;

public void CheckStatus(object stateInfo)
        {
            // Stop the timer while we do work
            Plugin.StateTimer.Change(Timeout.Infinite, Timeout.Infinite);

            // I assume the sessions are active session?
            var activeUsers = _sessionManager.Sessions.Select(s => s.UserName).ToArray();
            var messages = Plugin.Instance.Configuration.Messages.ToList();

            foreach (var message in messages)
            {
                if (!message.SendOnSessionStart)
                    continue;
               
                foreach (var activeUser in activeUsers)
                {
                    if (!message.Recipients.Contains(activeUser))
                        continue;
                   
                    _messageService.Post(message);
                    message.RemoveRecipient(activeUser);
                }
            }

            // Probably want to filter the messages that no longer have recipients
            var messagesWithRecipients = messages.Where(m => m.Recipients.Any()).ToList();
            
            Plugin.Instance.UpdateConfiguration(new PluginConfiguration { Messages = messagesWithRecipients });
        }

[chef 3745]

Many thanks. I didn't realize I could use collection and have the JavaScript serialize to it.

 

My headache has subsided.

 

I like how you used the read only property of _recipients to change and set back into the configuration.

 

I just had to watch out for the "if" statements which used "!" To continue the loop. Resharper always wants to make it false to continue, when it should be true in some circumstances.

 

I really appreciate the help.

 

Thank you.

[rhodges 37]

You only need to iterate backwards if you modify the size of the collection. The backwards for/index loop is the defacto standard. Usually the fastest since you are iterating it only once.

 

If you modify an item inside the list, you don't have to do it backwards.

[ebr 14912]

You only need to iterate backwards if you modify the size of the collection. The backwards for/index loop is the defacto standard. Usually the fastest since you are iterating it only once.

 

If you modify an item inside the list, you don't have to do it backwards.

 

Yes but a List object in c# is not ordered so there is no way to traverse it in any prescribed direction .

[chef 3745]

I actually found this very interesting. I love these guys!

 

 

Not really about removing things from a list, but more about iteration backwards and forwards through arrays and list in different styles of computers

Edited July 20, 2017 by chef

[rhodges 37]

Yes but a List<T> object in c# is not ordered so there is no way to traverse it in any prescribed direction .

 

I'm sure I'm not adding anything here that you don't already know @@ebr. Adding more for just community information.

 

We might be talking semantics at this point. I was referring to the Count/Index-- method of going backwards, which both List<T> and Arrays both provide.

 

Correct that a List<T> isn't ordered by anything other than the order in which it was filled. Of course that might all be abstracted away from you in the implementation (so who knows exact the order of the data in memory). A List<T> isn't the same as an Array even though most use them interchangeably.

 

However, since it has an indexer, you can iterate it (well, I guess you're not really iterating it, semantics, but rather decrementing an index) and thus safely remove elements so long as you are doing it backwards.

 

The for loop backwards to remove items from the list using the index is the defacto standard.

 

Of course, with Linq and all the extensions and deferred execution and the like, things can get tricky depending on what interface/class you are working with.