Like @DavidArno has correctly stated in his comment
And now you have a global method with static state, which is not a good place to be.
this isn't the best way to do this. By having a static dictionary this just can't be tested correctly, because the dictionary keeps its content between tests which is a no go.
A better way would be to create an interface having the GetAvatar()
method which is implemented by a simple class. The interface should then be constructor injected and stored in a private field.
public interface IAvatarService
{
string GetAvatar(System.Security.Principal.IIdentity user);
}
public class AvatarService : IAvatarService
{
private ConcurrentDictionary<string, string> avatarDictionary = new ConcurrentDictionary<string, string>();
public string GetAvatar(System.Security.Principal.IIdentity user)
{
if (user == null) { throw new ArgumentNullException("user"); }
string userId = user.GetUserId();
string avatar;
if (avatarDictionary.TryGetValue(userId, out avatar))
{
return avatar;
}
avatar = user.GetAvatar() // call of the extension method
avatarDictionary.AddOrUpdate(userId, avatar, (key, oldValue) => avatar);
return avatar;
}
}
having a expire mechanism would still be needed and should be implemented.