3
\$\begingroup\$

I have a manager class (included below) and sub data types that are included inside it. All of the different stored types are more or less the same with some wrapper functions calling the exact same named function below to the sub object. This feels dirty and I can't help but think there's a better way to do this. Suggestions?

using System;
using System.Collections.Generic;
using System.Linq;
using Inspire.Shared.Models.Enums;
using Lidgren.Network;

namespace GameServer.Editor.ContentLocking
{
    /// <summary>
    /// The content lock manager helps provide delegation
    /// </summary>
    public class ContentLockManager
    {

        // This is a mapping of all the different content lock stores available
        Dictionary<ContentType, ContentLockStore> _contentLockStores = new Dictionary<ContentType, ContentLockStore>();

        public ContentLockManager()
        {
            // Generate the ContentMap dynamically, assinging everyone a backing
            foreach (var contentType in GetValues<ContentType>())
                _contentLockStores.Add(contentType, new ContentLockStore());
        }

        /// <summary>
        /// Invalidates and purges all locks for a given connection.
        /// This is typically called when a connection has disconnected from the node.
        /// </summary>
        /// <param name="connection"></param>
        public void PurgeLocks(NetConnection connection)
        {
            foreach (var contentLockStore in _contentLockStores.Values)
            {
                contentLockStore.ReleaseLocks(connection);
            }
        }

        public List<int> GetLockedContent(ContentType contentType)
        {
            var contentStore = _contentLockStores[contentType];
            return contentStore.GetLockedContentIDs();
        }

        public bool HasLock(NetConnection connection, int ID, ContentType contentType)
        {
            var contentStore = _contentLockStores[contentType];
            return contentStore.HasLock(connection, ID);
        }

        public bool AnyoneHasLock(NetConnection connection, int ID, ContentType contentType)
        {
            var contentStore = _contentLockStores[contentType];
            return contentStore.AnyoneHasLock(connection, ID);
        }

        public bool TryAcquireLock(NetConnection connection, int ID, ContentType contentType)
        {
            var contentStore = _contentLockStores[contentType];
            return contentStore.TryAcquireLock(connection, ID);
        }

        public bool TryReleaseLock(NetConnection connection, int ID, ContentType contentType)
        {
            var contentStore = _contentLockStores[contentType];
            return contentStore.ReleaseLock(connection, ID);
        }

        private static IEnumerable<T> GetValues<T>()
        {
            return Enum.GetValues(typeof(T)).Cast<T>();
        }

    }
}

And then I have a mapping inside like so:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using Lidgren.Network;

namespace GameServer.Editor.ContentLocking
{
    /// <summary>
    /// A collection of keys internally of content that has been locked
    /// </summary>
    public class ContentLockStore
    {
        // Current locks on a particular piece of content
        private Dictionary<int, NetConnection> _contentLocks = new Dictionary<int, NetConnection>(); 

        public bool TryAcquireLock(NetConnection connection, int ID)
        {
            // Trying to aqquire a lock for something you already have or you're not authorized to obtain
            if (_contentLocks.ContainsKey(ID))
                return false;

            // Noone has taken it - go ahead and grab this
            _contentLocks.Add(ID, connection);

            return true;

        }

        /// <summary>
        /// Release sall locks with an associated contention
        /// </summary>
        /// <param name="connection"></param>
        public void ReleaseLocks(NetConnection connection)
        {
            var keys = _contentLocks.Where(cLock => cLock.Value == connection).ToList();
            keys.ForEach(x => ReleaseLock(x.Value, x.Key));

        }

        public bool ReleaseLock(NetConnection connection, int ID)
        {
            // If they don't have a lock, they can't release it
            if (!HasLock(connection, ID))
                return false;

            // Release the lock
            _contentLocks.Remove(ID);

            return true;

        }

        /// <summary>
        /// Returns a list of all the locked content IDs for a particular store
        /// </summary>
        /// <returns></returns>
        public List<int> GetLockedContentIDs()
        {
            return _contentLocks.Keys.ToList();
        }

        /// <summary>
        /// Determines whether a particular connection has a lock on a piece of content
        /// </summary>
        /// <param name="connection"></param>
        /// <param name="ID"></param>
        /// <returns></returns>
        public bool HasLock(NetConnection connection, int ID)
        {
            // If the key dosen't even exist, don't bother
            if (!_contentLocks.ContainsKey(ID))
                return false;

            return _contentLocks[ID] == connection;

        }

        public bool AnyoneHasLock(NetConnection connection, int ID)
        {
            return _contentLocks.ContainsKey(ID);
        }
    }
}
\$\endgroup\$
8
  • \$\begingroup\$ I'm not entirely sure I understand the relationship between the two classes. What does the constructor for ContentLockManager do? Are IDs always int? What other mappings do you have other than ContentLockStore? \$\endgroup\$
    – Bobson
    Aug 5, 2013 at 19:06
  • \$\begingroup\$ @Bobson The lock manager is creating a backing store for every ContentType and adding it to the dictionary. IDs are always ints - as defined. It's just a pure backing store. I need a ContentType locking store each ContentType. Basically, when a piece of content is opened I want it "locked" to that connection. This is done per content piece (by ID) and per content type. \$\endgroup\$ Aug 5, 2013 at 19:29
  • \$\begingroup\$ So, Manager -> list of all things that can be locked, and Store -> individual item's locks? And you have multiple Store classes, for each type of item? \$\endgroup\$
    – Bobson
    Aug 5, 2013 at 19:37
  • \$\begingroup\$ @Bobson That is correct. \$\endgroup\$ Aug 5, 2013 at 19:39
  • \$\begingroup\$ I think I'm getting it. Last question: Is there any change in the code between the various Store classes? I don't see anything in there which actually uses ContentType. Or is the question simply about having near-duplicate code in the Manager and Store classes? \$\endgroup\$
    – Bobson
    Aug 5, 2013 at 19:48

1 Answer 1

1
\$\begingroup\$

As per the comments and Your actual problem, it seems that You are unknowingly and partly using a Proxy Pattern.

So as a slight modification You should be having an interface extracted from ContentLockStore and then inherited in both of Your ContentLockManager and ContentLockStore classes, in Your case since the Manager class is having a third parameter as well, it is impossible to do even this, and hence partly implemented Proxy pattern.

I wonder whether Indexers can come to Your rescue, try reading them.

Besides Your main question, consider using Constructor Injection in Your code, Your Manager class's constructor should not be having that loop, a factory should should be creating a dictionary and then passing it in the constructor. Doing that will promote testability.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.