Game Permission System

I have just finished coding a permission system for my game server but one method in the class is seeming to be very complicated, and maybe some people on this site can help me improve the method?

Let me walk you through the permission system, which will give you a better understanding of the question, and how you can review my code.

The screenshot clearly explains a lot more just by looking at it.

PermissionManager.cs:

``
namespace MyApp.Permissions
{
using System;
using System.Collections.Generic;
using System.Linq;
using Players.Players;
using Utilities;

internal sealed class PermissionManager : IManager
{
private readonly Dictionary _permissions;

public PermissionManager()
{
_permissions = new Dictionary();
}

public void Initialize()
{
using (var databaseConnection = Hariak.HariakServer.Database.NewDatabaseConnection)
{
databaseConnection.SetQuery("SELECT * FROM game_permissions`");

using (var reader = databaseConnection.ExecuteReader())
{
while (reader.Read())
{
_permissions.Add(reader.GetInt32("id"), new Permission(
reader.GetString("permission_name"),
reader.GetString("ranks_allowed"),
reader.GetString("usernames_allowed"),
reader.GetInt32("minimum_rank"),
reader.GetInt32("maximum_rank"),
reader.GetString("user_ids_allowed"),
reader.GetString("custom_column_set")));
}
}
}
}

public bool CheckPermission(PlayerConnection playerConnection, string permissionName)
{
try
{
foreach (var permission in _permissions.Values.Where(permissi

From a purely MySQL perspective, I would comment that your schema does not seem appropriate.

Why reference both username AND user id for permissioning? Should user id be the authoritative identifier?

Why store users with given permission in a comma-separated list as opposed to using an actual many-to-many join table between permissions and users which would be a typically normalized approach? The way you have it now, you will have inefficient queries when looking up permissions for a given user id.

The same concern might be true about ranks_allowed, though it is unclear how this is used in your application.

Just generally your approach seems odd here in looking at your code. Normally, one might model permissions as being attached to a user object. That is when you marshall a user object for use in your system, you would also attach the permissions that user has within the system. These truly are properties of the user in a real-world sense. So it would seem to me that, if that were correctly modeled, all your permissions management code would need to do is to match those user permissions against the permissions required to perform some functionality.

The way you are doing it now is sort of the inverse. You look at the permission, and then determine if permission is appropriate for a given user, which just seems an odd way to model this, unless your primary use case was not to determine what an individual user has access to, but rather determining all users who have a permission as one might in the admin portion of an application.

My best advice, is to try to model your classes (and related database tables) in as close to a real world sense as you can. This typically helps simplify your code, and put the proper functionality in the proper classes.

StackExchange Code Review Q#158557, answer score: 11