.NET (Core and Framework)

Collin Jasnoch wrote:

Why does it need an object just for locking?

private object m_objVideoPlayerSync = new object();
private List<string> m_lstVideos = new List<string>();
private EnumPlaybackState m_playbackState = EnumPlaybackState.Stopped;

private void AddVideoToList(string szVideoFileName)
{
    lock(m_objVideoPlayerSync)
    {
        m_lstVideos.Add(szVideoFileName);
    }
}

private void PlayNextVideo()
{
    lock(m_objVideoPlayerSync)
    {
        string szVideo = m_lstVideos[0];
        m_lstVideos.RemoveAt(0);
        m_playbackState = EnumPlaybackState.Playback;
        StartPlayBack();
    }
}

private void StopVideo()
{
    lock(m_objVideoPlayerSync)
    {
        StopPlayBack();
        m_playbackState = EnumPlaybackState.Stopped;
    }
}

string szVideo = string.Empty;
lock(m_lstVideos)
{
    szVideo = m_lstVideos[0];
    m_lstVideos.RemoveAt(0);
}

lock(m_objVideoPlayerSync)
{
    m_playbackState = EnumPlaybackState.Playback;
    StartPlayBack();
}

In general, avoid locking on a public type, or instances beyond your code's control. The common constructs lock (this), lock (typeof (MyType)), and lock ("myLock") violate this guideline:

lock (this) is a problem if the instance can be accessed publicly.

lock (typeof (MyType)) is a problem if MyType is publicly accessible.

lock(“myLock”) is a problem since any other code in the process using the same string, will share the same lock.

Best practice is to define a private object to lock on, or a private static object variable to protect data common to all instances.

Luc Pattyn wrote:

All is fine if the outside world cannot touch the lock object, so if you have say a return _collection; somewhere, then the first is no good.

Luc Pattyn wrote:

For collections there should at least be an enumerator that allows modifications to the elements that have already been visited including removal from the collection, so one could e.g. easily remove elements that satisfy some conditions while enumerating them all.

1. Any item that exists throughout the enumeration will be returned exactly once in each pass through the enumeration.
   
2. Any item that is added and/or removed during the enumeration will be returned zero or one times (a removal and re-add may create a "new" item) in any pass.
   
3. For collections with keys, changing a key may be regarded as a remove and add, which may be performed in either order.
   
4. Items will be returned in a sequence consistent with the contract (e.g. if a particular collection is supposed to return items in sorted order, adding and deleting items should not cause items to be returned out of sequence).
   
5. If the effect of an addition or removal is observed in one pass through the enumeration, it will be observed in subsequent passes.
   

I guess I don't really see a whole lot of situations where the exception behavior is more useful than the behavior I described, especially since--if iEnumerator provided a collectionChanged property--one could code a generic class that, given an iEnumerator(of T), would implement iEnumerator(of T) but throw an exception if an effort was made to fetch an item after the underlying class had changed. Thus, if one wanted to have a "For Each" loop that would throw an exception if the collection changed, one could write:

For Each myThing as myClass in New ProtectedEnumerable(myCollection)
  ...
EndIf

Using MyNiceEnum as iNiceEnum(of MyThing) = myNiceCollection.GetNiceEnum
  Dim MyItem as MyThing
  While MyNiceEnum.GetNext
    MyItem = MyNiceEnum.TheItem
    .. Do something with MyItem
  End While
End Using

For Each MyItem in myNiceCollection
  .. Do something with MyItem
Next