C#: How to go through dictionary items without exposing dictionary as public?

Question

The class looks as follows:

public static class CacheManager
{
    private static Dictionary<string, object> cacheItems = new Dictionary<string, object>();

    private static ReaderWriterLockSlim locker = new ReaderWriterLockSlim();

    public static Dictionary<string, object> CacheItems
    {
        get
        {
            return cacheItems;
        }
    }
    ...
}

The ReaderWriterLockSlim locker object should be used as well.

The client looks as follows now:

foreach (KeyValuePair<string, object> dictItem in CacheManager.CacheItems)
{
    ...
}

Thank you in advance.

Java? You can expose a read-only view of the dictionary or the keys. Or look at the visitor pattern. — Thilo, Jun 14 '12 at 07:29
Is there any reason why you are not using a concurrent dictionary? — weismat, Jun 14 '12 at 07:37
whats the error you are getting? and what are you trying to achieve — Raghuveer, Jun 14 '12 at 07:37
@Raghuveer Guthikonda, There's no error. I just want to properly organize my class. — tesicg, Jun 14 '12 at 07:46
possible duplicate of [Is there a read-only generic dictionary available in .NET?](http://stackoverflow.com/questions/678379/is-there-a-read-only-generic-dictionary-available-in-net) — Chris, Jun 14 '12 at 07:54

score 6 · Accepted Answer · edited Jun 14 '12 at 07:52

6

If you only need to iterate the contents, then frankly it isn't really being used as a dictionary, but an iterator block and indexer might be used to hide the internal object:

public IEnumerable<KeyValuePair<string, object>> CacheItems
{
    get
    { // we are not exposing the raw dictionary now
        foreach(var item in cacheItems) yield return item;
    }
}
public object this[string key] { get { return cacheItems[key]; } }

edited Jun 14 '12 at 07:52

Rawling

49,248
7
89
127

answered Jun 14 '12 at 07:51

Marc Gravell

1,026,079
266
2,566
2,900

The class needs to be static. It seems I can use this without indexer? Right? – tesicg Jun 14 '12 at 07:57
@tesicg: So what? Make it static. Do you want to learn how to do it or do you want other people to program for you? – reinierpost Jun 14 '12 at 08:00
@reinierpost, No. As you can see the part of my code is already up there. But, I just commented I didn't need the indexer. That's all. – tesicg Jun 14 '12 at 08:03
@tesicg if you don't need access via index, then I question why it is a dictionary, but... whatever meets your requirements – Marc Gravell Jun 14 '12 at 08:13
@rawling thanks for the fix there... typed in a hurry before leaving my desk – Marc Gravell Jun 14 '12 at 08:14
@tesicg the reason I didn't make it static is that you should be **doubly** cautious about static fields and thread-safety. In some ways, I might suggest making it a `Hashtable` if it is static - which might sound crazy, but `Hashtable` has a better threading model than `Dictionary<,>`: it supports multiple readers **and** a single writer; meaning you only need to synchronize *mutate* code; your *read* code does not need any thread safety – Marc Gravell Jun 14 '12 at 08:19
@Marc Apologies for not at least leaving you the 5 minute window to fix it... I was in the middle of writing a similar answer so just jumped straight in :p – Rawling Jun 14 '12 at 08:22
@Marc Gravell, Just one more question - Do I need any synchronization object if I use your example? I use ReaderWriterLockSlim object in Add method. – tesicg Jun 14 '12 at 08:34
@tesicg if it is accessed from multiple threads, then yes; `Dictionary<,>` makes no guarantees about thread safety at all. – Marc Gravell Jun 14 '12 at 11:12

Oliver · Answer 2 · 2012-06-14T09:13:26.800

3

How about this property:

public static IEnumerable<KeyValuePair<string, object> CacheItems
{
    get
    {
        return cacheItems;
    }
}

The Dictionary implements the IEnumerable interface (which will already used by your foreach statement), but by only exposing it really as a IEnumerable you prevent any possibility to add or remove items to the dictionary.

If you need to access the dictionary by the index operator you can quite easily implement a ReadOnlyDictionary. It would then look something like this:

public class ReadOnlyDictionary<TKey, TValue> : IDictionary<TKey, TValue>
{
    private IDictionary<TKey, TValue> _Source;

    public ReadOnlyDictionary(IDictionary<TKey, TValue> source)
    {
        if(source == null)
            throw new ArgumentNullException("source");

        _Source = source;
    }

    // ToDo: Implement all methods of IDictionary and simply forward
    //       anything to the _Source, except the Add, Remove, etc. methods
    //       will directly throw an NotSupportedException.
}

In that case you could then also propagate your cache as

private static ReadOnlyDictionary<string, object> _CacheReadOnly;
private static Dictionary<string, object> _CacheItems;

public static ctor()
{
    _CacheItems = new Dictionary<string, object>();
    _CacheReadOnly = new ReadOnlyDictionary(_CacheItems);
}

public static IDictionary<string, object> CacheItems
{
    get
    {
        return CacheReadOnly;
    }
}

Update

If you really need to prevent the cast back to Dictionary you could also use this:

public static IEnumerable<KeyValuePair<string, object> CacheItems
{
    get
    {
        return cacheItems.Select(x => x);
    }
}

edited Jun 14 '12 at 09:13

answered Jun 14 '12 at 07:39

Oliver

43,366
8
94
151

1

watch out that this can still be cast back to the original dictionary – Marc Gravell Jun 14 '12 at 07:41
I've thought about that. Does that mean the dictionary is not exposed as public? – tesicg Jun 14 '12 at 07:43
@Marc Gravell, I agree. I think it's the same as I have in my original post. – tesicg Jun 14 '12 at 07:44
1

The problem with this is that it does actually return the dictionary object, just disguised as something else. You can cast it back to a Dictionary quite easily and then you basically have the original dictionary. – Chris Jun 14 '12 at 07:44
Marc is that usually considered a bad practice? I usually return "private" lists as IEnumerable and consider those to be read only. Are you saying one should always copy returned collections? – Anders Forsgren Jun 14 '12 at 07:44
@tesicg it isn't quite the same: it isn't as *obvious*, but the object still is exposed. – Marc Gravell Jun 14 '12 at 07:49
@AndersForsgren it really depends on how much you are concerned about the caller, and whether they could cause harm by casting the type into a dictionary such as `IDictionary<,>` – Marc Gravell Jun 14 '12 at 07:49
@Marc Gravell, So, it's ok to do it in the way as Oliver wrote? – tesicg Jun 14 '12 at 07:50
Oliver: regarding your update the test code I just wrote (in linqpad) still seems to be able to cast that back to dictionary... – Chris Jun 14 '12 at 07:58
@MarcGravell and in the the case where you are wrapping a private dictionary with and IDictionary you can use reflection to get at the wrapped dictionary. But both cases are a contract violation which should remove the expectation that program will continue to perform correctly. – Yaur Jun 14 '12 at 08:04
@tesicg as I wrote, that depends on how much you care about the risk of it being cast, and a caller changing the contents without you expecting it – Marc Gravell Jun 14 '12 at 08:16
@Yaur there's a bit of a difference between reflection (which clearly violates all contracts, and can be prevented depending on the security context), and a simple: `var lookup = data as IDictionary; if(lookup != null) {...}` – Marc Gravell Jun 14 '12 at 08:17
@MarcGravell I hear you. But also depends on *what* I'm concerned about I suppose. A stupid user or a malicious one. A malicious user will get my private list using reflection, no casting required. Stupid user might cast the IEnumerable to List and clear it. One can prevent the second... – Anders Forsgren Jun 14 '12 at 12:53
@AndersForsgren Indeed. One slight complication there is that a lot of library could might take `object` and check, for example, for `IList` etc (and other common interfaces). – Marc Gravell Jun 14 '12 at 12:58

score 1 · Answer 3 · edited Jun 14 '12 at 09:17

1

Expose a property that is a read only view on the dictionary that filters? Expose methods that allow access to the itemas as possible.

What exactly is the problem? "without exposing dictionary as public" is vague enough that this is not answerable.

edited Jun 14 '12 at 09:17

AaA

3,600
8
61
86

answered Jun 14 '12 at 07:37

TomTom

61,059
10
88
148

"without exposing dictionary as public" means replace CacheItems property with something else. – tesicg Jun 14 '12 at 07:45
@tesicg so you want the dictionary (and its contents) to be read only? – Trisped Jun 14 '12 at 07:46
@Trisped, Yes. And I've thought I did it with get clause in my original post. – tesicg Jun 14 '12 at 07:49
get makes dictionary readonly but not its content. I agree with TomTom, you might consider putting your iterator method inside your class and only expose the dictionary search result (if exists). – AaA Jun 14 '12 at 07:53
2

Also I believe Tomtom should get a slapped wrist for comment as answer. at 20k rep I would have thought you knew better. :) – Chris Jun 14 '12 at 07:59

score 0 · Answer 4 · answered Jun 14 '12 at 08:01

0

Depending on the size of the dictionary you could use MemberwiseClone.

You might also want to look into the IsReadOnly property.

answered Jun 14 '12 at 08:01

Trisped

5,705
2
45
58

C#: How to go through dictionary items without exposing dictionary as public?

4 Answers4

Update