DataService data storage

Question

I have a DataService, which should talk to the server and store vars (for complicated calls, for example).

Last time, I ended up with this approach (for the example, I'm working with a list):

• GetList() - returns cached data (just a property)

• SetList() - sets a property

• ReadList() - read data from file from IsoStorage

• WriteList() - write data to file

• PullList() - pull data from the server

• PushList() - push data to the server

NB: I'm using my old IsoStorage approach (IsoSettingsManager).

Now, after some time, I understood that I can merge first two stages: caching and writing to IsoStorage. Now I'm thinking of something like

    private int? deliveryId = null;
    public int DeliveryId
    {
        get { return deliveryId ?? IsoSettingsManager.GetProperty<int>("DeliveryId"); }
        set { deliveryId = value; IsoSettingsManager.SetProperty("DeliveryId", value); }
    }

Where setting property is

public static void SetProperty(string propertyName, object content)
    {
        if (System.ComponentModel.DesignerProperties.IsInDesignTool)
            return;

        if (content == null)
            RemoveProperty(propertyName); // if (isoSettings.Contains(propertyName)) isoSettings.Remove(propertyName);

        isoSettings[propertyName] = content;
        isoSettings.Save();
    }

What do you think about it?

First thing that comes to my mind is to make SetProperty to return the setted value so I'd be able to use it like

public int DeliveryId
{
    set { deliveryId = IsoSettingsManager.SetProperty("DeliveryId", value); }
    ***

unsetting DeliveryId = null; is not working for ints (fair enough), and I couldn't yet find how to push/pull them effectively.

Answer 1

Style

It won't hurt you and makes your code less error prone, if you would use braces {} also for single if statements.

Commented code (// if (isoSettings.Contains(propertyName)) isoSettings.Remove(propertyName);) should be removed completely. You should instead use a code versioning system like svn or git.

Update based on the comments below
It is not about a comment but about commented out code which is just dead code. What value has dead code ? Does it make your live code more readable or does it make your live code easier to understand ? No, it is quite the opposite.

Asume the following

public static void SetProperty(string propertyName, object content)
{
    //if (System.ComponentModel.DesignerProperties.IsInDesignTool)
    //    return;

    //if (content == null)
        RemoveProperty(propertyName); // if (isoSettings.Contains(propertyName)) isoSettings.Remove(propertyName);

    isoSettings[propertyName] = content;
    isoSettings.Save();
}  

this is quite hard to read and understatnd by Mr.Maintainer.

This instead

public static void SetProperty(string propertyName, object content)
{
    RemoveProperty(propertyName);

    isoSettings[propertyName] = content;
    isoSettings.Save();
}

is much better to understand.

If you check something into svn/git you always should add a descriptive message to the commit.

SetProperty() method

if (content == null)
        RemoveProperty(propertyName); // if (isoSettings.Contains(propertyName)) isoSettings.Remove(propertyName);

    isoSettings[propertyName] = content;
    isoSettings.Save();  

this does not make sense to me. If content==null you first remove the property, and then you add it. Next time passing null you just do the same.

You should consider returning after you have removed the property. There is no need to save the setting.

SetProperty() returning setted value

This is in my opinion a bad idea for a method which seems to be like a property setter. A property setter should only set a properties value.