2
\$\begingroup\$

This question came up today on StackOverflow. I find it more unfortunate than (as some others said) idiotic. And it recalled an old function I have which I just remembered.

I have a user.config file which has some settings which can be updated through user action. My rationale was "if the setting is changed, it should be saved." So I made a static class to cover for those settings so as not to handle the operations each time. And I came up with this:

public static class UserSettings
{
    public static string ProcessPath
    {
        get { return _processPath; }
        set 
        { 
            _processPath = Settings.Default.ProcessPath = value;
            Settings.Default.Save();
        }
    }
    //Other functions and properties
}

Yes, I know, it reeks. I feel shame. I want to make this right. I'm thinking this:

  • the class is static, it should at least be a singleton;
  • the property should itself not be static;
  • there's no point in saving the settings every time, just on closing the app, and that would basically be safe in a destructor for the class be best suited for a dedicated function called SaveSettings. Any other object can call it when they see fit, no surprise effects. (further evidence of the need for a singleton).

Are these good intentions in the right direction?

\$\endgroup\$
  • 3
    \$\begingroup\$ Oh my, that question you linked to just killed a part of my soul. Ouch. Your example however, I don't really have a problem with. \$\endgroup\$ – Jeff Mercado Sep 28 '11 at 18:54
  • \$\begingroup\$ You are welcome to see my post then. You are almost right, there is no troubles using application/user scope with settings along with singletone implementation in .NET. Just use singleton serialization helper and serializatpion class, inherited from ISerializable interface, and make use of the Default static property to access your serialized properties. \$\endgroup\$ – Artur Mustafin Sep 29 '11 at 7:11
1
\$\begingroup\$

First, I think making it a singleton would be a mistake (singleton usually is). At best it would be extra work with zero benefit. At worst, it's a handicap, such as when/if you decide to support two top-level windows with independent settings for each.

Second, I think I'd add a Boolean named dirty (or something similar). When you set the value, dirty is set to true. When you save the value, you check if dirty is true, and only write out the value if it is (then you set dirty to false).

Personally, I'd probably change it from a static class named ProcessPath to a normal class named (something like) config_string, with an instance for ProcessPath, and a the possibility for other instances holding other strings. To do that, you'd pass Settings.Default.ProcessPath as a parameter when you construct the object.

\$\endgroup\$
  • 1
    \$\begingroup\$ Why do you say singletons usually are mistakes? \$\endgroup\$ – MPelletier Sep 28 '11 at 20:13
  • 1
    \$\begingroup\$ @MPelletier: If you Google for something like singleton antipattern, you should get some pretty decent ideas. \$\endgroup\$ – Jerry Coffin Sep 28 '11 at 20:19
  • \$\begingroup\$ A dirty for every value? What if a class contains 100 values? \$\endgroup\$ – Brian Sep 28 '11 at 21:14
  • 1
    \$\begingroup\$ @0A0D: Right now it contains one value. If it contains 100 values, it will depend. If each can be/is reasonably written out separately, then probably yes. If they're all going to be written together, then a single Boolean suffices for all of them. Keep in mind that 100 bits is only 12.5 bytes. Even if you round up to 13 or even 16 (next power of 2), and it's still pretty minimal. You don't have to save many writes to disk to justify 16 bytes of storage. \$\endgroup\$ – Jerry Coffin Sep 28 '11 at 22:02
  • \$\begingroup\$ @0A0D @Jerry Actually, since this is basically a cover class for user.config settings, Settings.Default.Save(); saves all values, so a single dirty variable would suffice. \$\endgroup\$ – MPelletier Sep 28 '11 at 23:58
1
\$\begingroup\$

I understand your idea. You have to use both patterns: singleton pattern, and load/save pattern.

You can use UserSettings class implementation, with load/save functionality, inherited from the ApplicationSettingsBase class.

Example of using UserSettings singleton class in Windows Forms Application:

public partial class Form1 : Form
{
    public Form1()
    {
        InitializeComponent();
    }
    private void Form1_Load(object sender, EventArgs e)
    {
        this.Location = UserSettings.Default.FormLocation;
        this.Size = UserSettings.Default.FormSize;
        this.textBox1.Text = UserSettings.Default.ProcessPath;
    }

    private void Form1_FormClosing(object sender, FormClosingEventArgs e)
    {
        UserSettings.Default.FormLocation = this.Location;
        UserSettings.Default.FormSize = this.Size;
        UserSettings.Default.ProcessPath = this.textBox1.Text;
        UserSettings.Default.Save();
    }
}

Sources:

[SettingsSerializeAs(SettingsSerializeAs.Xml)]
public sealed class UserSettings : ApplicationSettingsBase, INotifyPropertyChanged, ISerializable
{
    private static UserSettings _defaultInstance = new UserSettings();

    [SecurityPermissionAttribute(SecurityAction.LinkDemand, Flags = SecurityPermissionFlag.SerializationFormatter)]
    void ISerializable.GetObjectData(SerializationInfo info, StreamingContext context)
    {
        info.SetType(typeof(SingletonSerializationHelper));
    }

    private UserSettings() { }

    public static UserSettings Default { get { return _defaultInstance; } }

    private const string FormLocationProperty = "FormLocation";
    private const string FormSizeProperty = "FormSize";
    private const string ProcessPathProperty = "ProcessPath";

    // public properties
    [UserScopedSetting()]
    [DefaultSettingValue("0, 0")]
    public Point FormLocation
    {
        get { return (Point)(this[FormLocationProperty]); }
        set { this[FormLocationProperty] = value; }
    }

    [UserScopedSetting()]
    [DefaultSettingValue("300, 300")]
    public Size FormSize
    {
        get { return (Size)this[FormSizeProperty]; }
        set { this[FormSizeProperty] = value; }
    }

    [UserScopedSetting]
    [DefaultSettingValue("")]
    public string ProcessPath
    {
        get { return (string)this[ProcessPathProperty]; }
        set { this[ProcessPathProperty] = value; }
    }
}

[Serializable]
internal sealed class SingletonSerializationHelper : IObjectReference
{
    public object GetRealObject(StreamingContext context)
    {
        return UserSettings.Default;
    }
}
\$\endgroup\$
  • \$\begingroup\$ I see what you did with the singleton pattern. Impressive, I had always just seen GetInstance(), never an actual named oject. I guess it tells us the original Properties.Settings.Default is itself a singleton... Your class would serialize (and deserialize) as user.config? What would be the advantage here? \$\endgroup\$ – MPelletier Sep 29 '11 at 9:56
  • \$\begingroup\$ @MPelletier: There is no real advantage of using this class except it is a singleton and is an ancestor of the ApplicationSettingsBase class, and can be used anywere ApplicationSettingsBase is used. So it solves the problem of multiple instancies of applications settings classes, in my opinion. Persistent storage, accessible anywere, which also has default values set, etc. \$\endgroup\$ – Artur Mustafin Sep 29 '11 at 13:37
  • \$\begingroup\$ I think I see... A lot of that stuff I don't need in the current project. Way too much overhead. But in the right place, it would be very useful. I'm just not sure where the right place would be at the moment. That is by no means a way of saying I don't like your solution. I do like it. I'm just not sure I understand all of it :S \$\endgroup\$ – MPelletier Sep 29 '11 at 14:16
  • 1
    \$\begingroup\$ @PMelletier: It's OK. This is a community. I got another solutions, which you can find in my careers profile, f.e. you can use custom XML serialization to serialize anything you want at xmlserialization.codeplex.com, etc. \$\endgroup\$ – Artur Mustafin Sep 30 '11 at 2:35
0
\$\begingroup\$

Here is the cheats I use -

To prevent having to type "Settings.Default":

using Res = MyApp.Properties.Resources;
using Set = MyApp.Properties.Settings;

/// in class
private static Set def{
  get{
    return Set.Default;
  }
}
// usage
def.Data="blah"

And then take a look at

public event SettingChangingEventHandler SettingChanging

of ApplicationSettingsBase

\$\endgroup\$

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.