Is there a simpler way than using this Singleton to clear the cache?

[h***@gmail.com]

We have a security wrapper that goes around all our MVC projects. We start first with the security wrapper because it has all the stuff concerning Roles that should have been made simpler by MS by now (this is just my opinion, I'm sure no one else thinks this. . .).

Now the person before me that build this wrapper included a call to Singleton to clear the cache if it was causing problems. It comes off the shelf without any cache items configured.

But I am almost sure this Singleton could be replaced with something simpler for just clearing the cache by the admin pushing a button. I've inherited the responsibility of maintaining and improving this security wrapper. Surely there is something simpler.


====================================
In the View there is an Action Link
====================================

@Html.ActionLink("Refresh Cache", "Refresh", "CacheManagement", new { @class = "btn btn-default" })

============================
that calls the ActionResult
============================

[HttpGet]
public ActionResult Refresh()
{
Singleton.GetInstance().RefreshCache();

ViewBag.Text = "Refreshed";
return View("Index");
}
========================
That runs the Singleton
========================

namespace security6wrapper.Utilities
{
public sealed class Singleton
{
private static Singleton _instance;
private string _value;
private readonly object _lock = new object();

//Constructor
private Singleton()
{
_value = string.Empty;
}

/// <summary>
/// Get an instance
/// </summary>
/// <returns></returns>
public static Singleton GetInstance()
{
return _instance ?? (_instance = new Singleton());
}

/// <summary>
/// Get Value
/// </summary>
/// <returns></returns>
public string GetValue()
{
return _value;
}

/// <summary>
/// Clears the cache;
/// </summary>
//[Authorize(Roles = Security.AdminRole)]
public void RefreshCache()
{
lock (_lock)
{
//reset all stored objects
_value = string.Empty;
}
}
}
}

[Arne Vajhøj]

Post by h***@gmail.com
Now the person before me that build this wrapper included a call to
Singleton to clear the cache if it was causing problems. It comes off
the shelf without any cache items configured.
But I am almost sure this Singleton could be replaced with something
simpler for just clearing the cache by the admin pushing a button. I've
inherited the responsibility of maintaining and improving this security
wrapper. Surely there is something simpler.
====================================
In the View there is an Action Link
====================================
@Html.ActionLink("Refresh Cache", "Refresh", "CacheManagement", new { @class = "btn btn-default" })
============================
that calls the ActionResult
============================
[HttpGet]
public ActionResult Refresh()
{
Singleton.GetInstance().RefreshCache();
ViewBag.Text = "Refreshed";
return View("Index");
}

You could use Application object instead of the custom singleton.

But the code does not look so bad.

The singleton itself on the other hand ...

Post by h***@gmail.com
========================
That runs the Singleton
========================
namespace security6wrapper.Utilities
{
public sealed class Singleton

The class should have a name that indicate what it is - in
this case WhateverCache or something like this.

Post by h***@gmail.com
{
private static Singleton _instance;
private string _value;
private readonly object _lock = new object();
//Constructor
private Singleton()
{
_value = string.Empty;
}
/// <summary>
/// Get an instance
/// </summary>
/// <returns></returns>
public static Singleton GetInstance()
{
return _instance ?? (_instance = new Singleton());
}

That is not thread safe.

Post by h***@gmail.com
/// <summary>
/// Get Value
/// </summary>
/// <returns></returns>
public string GetValue()
{
return _value;
}
/// <summary>
/// Clears the cache;
/// </summary>
//[Authorize(Roles = Security.AdminRole)]
public void RefreshCache()
{
lock (_lock)
{
//reset all stored objects
_value = string.Empty;
}
}
}
}

Arne

[Marcel Mueller]

Post by h***@gmail.com
We have a security wrapper that goes around all our MVC projects. We start first with the security wrapper because it has all the stuff concerning Roles that should have been made simpler by MS by now (this is just my opinion, I'm sure no one else thinks this. . .).
Now the person before me that build this wrapper included a call to Singleton to clear the cache if it was causing problems. It comes off the shelf without any cache items configured.
But I am almost sure this Singleton could be replaced with something simpler for just clearing the cache by the admin pushing a button. I've inherited the responsibility of maintaining and improving this security wrapper. Surely there is something simpler.

First of all you should check the purpose of the Singleton class.
If the code was complete the class Singleton is meaningless in every
respect.
_value and therefor GetValue() will always be the same: string.Empty.
And since all program string literals are deduplicated even
object.ReferenceEquals will always return true.
In this case you can drop the entire class including all RefreshCache
calls and replace all references to GetValue by "".

If there is further code in class Singleton that might assign other
values to _value you should at first implement the singleton pattern
correctly. The current implementation has a race condition.
The correct C# implementation is:
static readonly Singleton Instance = new Singelton();


Marcel

[Arne Vajhøj]

Post by Marcel Mueller
If there is further code in class Singleton that might assign other
values to _value you should at first implement the singleton pattern
correctly. The current implementation has a race condition.
static readonly Singleton Instance = new Singelton();

I think is *a* correct implementation not *the* correct
implementation.

Arne