Architecture to cache data

Question

I cache the data and use local database in my Windows Phone app. The algorithm is very simple:

1. Get data from DB and show in UI
2. Get data from a web service and show in UI
3. Update data in DB from the web service

Some data need to be saved in DB, and some do not need to.

For these purposes, I have the following classes.

GYDataAccessLayer is an entry point. This class sets and changes the data source.

Example:

GYDataAccessLayer da = new GYDataAccessLayer();
da.GetInfo((result, error) => 
 {
    if (error != null) { return; } // handling exception
    GYUser user = result; 
 });


public class GYDataAccessLayer
{
    public void GetUserInfo(Action<GYUser, Exception> act, string id)
    {
        GYDataSource dataSource = new GYDataSource(new GYLocalData());
        dataSource.GetUserInfo(act, id);

        dataSource.SetDataSource(new GYWebData());
        dataSource.GetUserInfo(act, id);
    }
}


public class GYDataSource
{
    IGYDataAccess _gyDataAccess;

    public GYDataSource(IGYDataAccess gyDataAccess)
    {
        _gyDataAccess = gyDataAccess;
    }

    public void SetDataSource(IGYDataAccess gyDataAccess)
    {
        _gyDataAccess = gyDataAccess;
    }

    public void GetInfo(Action<GYUser, Exception> act, string uid)
    { 
        _gyDataAccess.GetUserInfo(act, uid);
    }
}

I have a two classes to work with Local and Web data source. GYWebData gets data from web and calls update method in GYLocalData.

public interface IGYDataAccess
    {
        void GetUserInfo(Action<GYUser, Exception> act, string id);         
    }

public class GYWebData : IGYDataAccess
{
    public void GetUserInfo(Action<GYUser, Exception> act, string id)
    {
        GYUserAPI.GetInfo(id, (result, error) =>
            {
                if (error != null)
                {
                    act.Invoke(null, error);
                    return;
                }

                act.Invoke(result.Result, null);
                GYLocalData gyLocalData = new GYLocalData();
                gyLocalData.UpdateUserInfo(result.Result);

            });
    }
}

 public class GYLocalData : IGYDataAccess
{
    private const string ConnectionString = @"isostore:/Cache.sdf";
    CacheDataContext DataBase;      
    CacheDataContextProfiles DataBaseProfiles;       

    private void CheckDataBase()
    {
        using (DataBase = new CacheDataContext(ConnectionString))
        {
            if (!DataBase.DatabaseExists())
            {
                DataBase.CreateDatabase();
            }
        }
    }


    public void GetUserInfo(Action<GYUser, Exception> act, string id)
    {
        CheckDataBase();

        using (DataBaseProfiles = new CacheDataContextProfiles(ConnectionString))
        {
            try
            {
                var user = DataBaseProfiles.GetInfo(id);
                if (user != null)
                {
                    act.Invoke(user, null);
                }
            }
            catch (Exception ex)
            {

                act.Invoke(null, ex);
            }
        }
    }


   public void UpdateUserInfo(GYUser user)
    {
        CheckDataBase();

        using (DataBaseProfiles = new CacheDataContextProfiles(ConnectionString))
        {
            DataBaseProfiles.UpdatePersonProfile(user);
        }
    }
 }

With this architecture it is inconvenient to work and hard to support. You have to write a lot of code and be very attentive (eg do not forget to call the update data method in the DB). Are there good patterns for such problems? How can I improve this code?

Answer 1

Let's go over the basics first!

The indentation is quite a mess. But I suppose that might be related to the SE formatting. Otherwise, you should make your indentation coherent. Stuff like :

public interface IGYDataAccess
    {
        void GetUserInfo(Action<GYUser, Exception> act, string id);         
    }

shouldn't be in production code, it looks bad and gives headaches to maintainer (that could be you!)

Why are your classes prefixed of GY? Maybe there's a good reason, but I'm pretty sure your names would make just as much sense without that prefix. The pattern you're using for caching uses the Repository design pattern. It abstracts the implementation to access to data. So that pattern name should be found in your naming! IRepository, LocalDataRepository, WebDataRepository. Some might argue that it's useless, but I think it's good to see the pattern's name in the class name, so we know what we're dealing with.

Multi-line lambdas are... okay, I guess. But I think in your case they should be in a method. A lambda is an anonymous function, something that doesn't matter that much in your code. But that code is important, don't let it crawl in the shadows, that code deserves a method. (The lambda in GetUserInfo)

Don't keep your connection string as a const in your code, that's a very bad practice. It should be in a configuration file, and you should get it using the ConfigurationManager class (In System.Configuration).

private members should be camelCased, not PascalCased, so DataBase -> dataBase (In GYLocalData). Also, you should specify the private accessor, so I know it wasn't forgotten or something like that. See, if it's not there. I don't know your clear intent. Maybe you wanted this field to be public but you forgot because you were out of coffee. I'll never know because you didn't specify it.

I want to give a little warning that might be unnecessary. The CheckDatabase method isn't thread safe. That means if your GYLocalData object can be called from multiple threads at once, you might create the database twice. And that's a costly operation. That's bad.

You should only use var when you can figure the Type by reading the code. In the example below, I can't figure it out, so it's harder to know what's happening and even harder to review! :)

var user = DataBaseProfiles.GetInfo(id);

You could use some Dependancy Injection in your GYWebData class. You want to store everything that is took from the web API in a another storage. So the GYWebdata class should receive in the constructor a GYLocalData instance where you would store the information :

public class GYWebData : IGYDataAccess
{
    private GYLocalData storage;

    public GYWebData(GYLocalData storage)
    {
        this.storage = storage;
    }

    public void GetUserInfo(Action<GYUser, Exception> act, string id)
    {
        GYUserAPI.GetInfo(id, (result, error) =>
            {
                if (error != null)
                {
                    act.Invoke(null, error);
                    return;
                }

                act.Invoke(result.Result, null);
                storage.UpdateUserInfo(result.Result);
            });
    }
}

Now this isn't exactly Dependency Injection because I give a concrete class as parameter. You would need another interface, IDataUpdater, maybe :

public interface IDataUpdater
{
   void UpdateUserInfo(GYUser user);
}

The LocalData class should implement that interface, and then :

public class GYWebData : IGYDataAccess
{
    private IDataUpdater updater;

    public GYWebData(IDataUpdater updater)
    {
        this.updater = updater;
    }

    public void GetUserInfo(Action<GYUser, Exception> act, string id)
    {
        GYUserAPI.GetInfo(id, (result, error) =>
            {
                if (error != null)
                {
                    act.Invoke(null, error);
                    return;
                }

                act.Invoke(result.Result, null);
                updater.UpdateUserInfo(result.Result);
            });
    }
}

Boom, you now have zero dependency between the local storage and the web storage.

Finally, looking at your workflow, you could use the Proxy design pattern to help a bit. The Proxy would be used to get info from the web and cache it in the local storage. The proxy would be the only class you use for storage. I don't want to give it all to you, because you'll learn better by yourself! :) But know that the Proxy is a good case for this!