Cookie repository for self-hosted web service

I use this in a self hosted web service, so I am not using a ton of libraries. I wanted a simple yet easy-to-use cookie repository. The reason the repository is static is that I wanted the ability to use the repository within Razor templates without having to use a reference object. I just find it simpler.

I feel it's pretty well-rounded in functionality as it stands right now, but a review is always nice.

```
using System;
using System.Collections.Generic;
using System.Linq;

namespace CookieRepository
{
class Program
{
static void Main(string[] args)
{
CookieRepository.Add("myid", "Authenticated", true);

// do something...

// Check cookie again.
if (CookieRepository.ValueEquals("myid", "Authenticated", true))
{
Console.WriteLine("Value was true, user is Authenticated.");
}
else
{
Console.WriteLine("Value was false, user is NOT Authenticated.");
}

// do something...

// Update cookie.
CookieRepository.Set("myid", "Authenticated", false);

// do something...

// Lets do another check.
if (CookieRepository.ValueEquals("myid", "Authenticated", true))
{
Console.WriteLine("Value was true, user is Authenticated.");
}
else
{
Console.WriteLine("Value was false, user is NOT Authenticated.");
}

Console.WriteLine("Press any key to exit...");
Console.ReadKey();
}
}

public static class CookieRepository
{
private static readonly HashSet> Cookies = new HashSet>();

///
/// Used when you want to add a cookie and being sure that cookie by same Key does not already exist.
///
/// Session Id of owner
/// Cookie known as
/// Value of the cookie
///

Your implementation of the CookieRepository is well written, easy to read and has good documentation, but it still can be beautyfied.

Style comments

• variables should be named using camelCase casing so the Cookies variable should be cookies. This should be done for method variables too. See the C# naming guidelines.

• method parameters should be named using camelCase casing too.

• IMHO braces {} should always be used and it won't hurt if you use them also where you put the execution statement on the same line like in the Equals() method.

The meat

The locking on an object which is involved in the processing is considered bad practice. You should always use a separate object to lock.

So replacing lock(Cookies), by using

private static readonly object cookieLock = new object();

, with lock(cookieLock) would be a good alternative to using the Cookies object.

public static bool Add(string SessionId, string Key, T Value)
{
    lock (Cookies)
    {
        if (Cookies.Any(x => x.SessionId == SessionId && x.Key == Key))
        {
            return false;
        }
        else
        {
            Cookies.Add(new Cookie(SessionId, Key, Value));
        }

        return true;
    }
}

here the else is redundant and should be removed, because if the condition is true the else won't be reached. By removing the else like so

public static bool Add(string SessionId, string Key, T Value)
{
    lock (Cookies)
    {
        if (Cookies.Any(x => x.SessionId == SessionId && x.Key == Key))
        {
            return false;
        }

        Cookies.Add(new Cookie(SessionId, Key, Value));
        return true;
    }
}

will also save horizontal spacing, hence your code is more readable.

public static void RemoveAll(string SessionId)
{
    lock (Cookies)
    {
        var SessionCookies = Cookies.Where(x => x.SessionId == SessionId).ToList();

        foreach (var Cookie in SessionCookies)
        {
            Cookies.Remove(Cookie);
        }
    }
}

By using the builtin RemoveWhere() method of the HashSet performance will be increased and it will result in less code.

public static void RemoveAll(string SessionId)
{
    lock (Cookies)
    {
        Cookies.RemoveWhere(x => x.SessionId == SessionId);
    }
}

/// 
/// Adds or Replaces a cookie, used when ensurance that cookie by key does not already exist is of any importance.
/// 
/// Session Id of owner
/// Cookie known as
/// Value of the cookie
public static void Set(string SessionId, string Key, T Value)

Instead of stating in the summary Adds or Replaces a cookie you should name the method AddOrReplace() to make it clear what the method does.

StackExchange Code Review Q#99212, answer score: 3