7
\$\begingroup\$

Following from this question

I created a Image cache of sorts (let's call it version 1.0) that used WeakReference as "cache" items. And this comes from the accepted answer:

It looks like weak reference would serve the purpose of what you are trying to achieve [IF] those images will go out of scope (not being directly referenced in any execution path) after handing them back to the caller of that method then they will be collected by GC at some point.

Here is the code that references on any way the images:

Note: cardBaseId and level are used to identify unambiguously a card, and thereby its image.

On the XAML:

<Image x:Name="Portrait" Style="{StaticResource Portrait}">
    <Image.Source>
        <MultiBinding Converter="{StaticResource PortraitConv}">
            <Binding Path="CardBaseId" />
            <Binding Path="MaxLevel" />
        </MultiBinding>
    </Image.Source>
</Image>

On the IValueConverter:

    public object Convert(object[] values, Type targetType, object parameter, System.Globalization.CultureInfo culture)
    { 
        int cardBaseId = (int)values[0];
        int level = (int)values[1];
        return ImageManagerInstance.GetImage(cardBaseId, level);
    }

On ImageManager.cs:

    public BitmapImage GetImage(int cardBaseId, int level)
    {
        Debug.Assert(FilepathManagerInstance != null); 

        string filepath = FilepathManagerInstance.GetImageFilepath(cardBaseId, level); 
        return new BitmapImage(new Uri(filepath));
    }

So I'd say that the images are not on the scope of my code anymore once they are sent to the XAML.

Still, I've changed the implementation to use MemoryCache as sugested for the reason above, and because this way I have more control of how memory is managed.

Here is the new code:

public class ImageCache
{
    private string Filepath;
    private MemoryCache Cache;
    private TimeSpan SlidingExpiration;

    public ImageCache(string filepathToStoredImages, int cacheMemoryLimitMegabytes, TimeSpan slidingExpiration)
    {
        Filepath = filepathToStoredImages;
        NameValueCollection config = new NameValueCollection();
        config.Add("CacheMemoryLimitMegabytes", cacheMemoryLimitMegabytes.ToString());
        Cache = new MemoryCache("ImageCache", config);
        SlidingExpiration = slidingExpiration;
    }

    public BitmapImage GetImage(int id)
    {
        string identifier = id.ToString();
        BitmapImage image = Cache.Get(identifier) as BitmapImage;

        if (image == null)
        {
            image = GetFromDisk(id);
            CacheItemPolicy policy = new CacheItemPolicy();
            policy.SlidingExpiration = SlidingExpiration;
            Cache.Set(identifier, image, policy);
        }

        return image;
    }

    private BitmapImage GetFromDisk(int id)
    {
        Uri uri = new Uri(Filepath + id + ".png");
        return new BitmapImage(uri);
    }
}  

Is the first time I use the MemoryCache class so any insight if this code is correct or something should be changed or improved?

\$\endgroup\$
1
\$\begingroup\$

Just a small thing: consider using object and collection initializers.

So this:

NameValueCollection config = new NameValueCollection();
config.Add("CacheMemoryLimitMegabytes", cacheMemoryLimitMegabytes.ToString());

Becomes (also using var):

var config = new NameValueCollection { { "CacheMemoryLimitMegabytes", cacheMemoryLimitMegabytes.ToString() } };

And this:

CacheItemPolicy policy = new CacheItemPolicy();
policy.SlidingExpiration = SlidingExpiration;

Becomes:

var policy = new CacheItemPolicy { SlidingExpiration = SlidingExpiration };

And if you do that, you might want to get rid of the variables, and just specify the value of the parameter directly.

Also, it's common to use camelCase, not PascalCase, for private fields.

\$\endgroup\$
1
\$\begingroup\$

I have read that Android used to use Weak/Soft Reference to cache image but they do not recommend this anymore as the garbage collector became more agressive over the time to better handle memory.

I see using WeakReference in C# possible to go the same route one day. Maybe you should consider that.

\$\endgroup\$
  • \$\begingroup\$ This is a reasonable concern, though, despite the text, I don't think the OP is actually using WeakReference (MemoryCache rather we might assume is well implemented). There is an interesting use-case for weak references in the context of caching outlined over on SO (multiple answers) that is worth a quick read. \$\endgroup\$ – VisualMelon Aug 14 '17 at 8:24

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.