Naming Conventions
- Avoid java naming conventions
void save
. Use C# conventions instead void Save
.
- Favor verbs
Initialise
over abbreviations Init
for method names.
Type Conventions
- Don't use
struct
for mutable objects. Use class
instead.
Design Considerations
Others have already pointed out Settings
could be used as a facade for your application, splitting the interface from the internal implementation. Ideally, you would want to create an interface for your settings providers and have one implementation that uses the auto-generated Properties.Settings
class. For the sake of limiting our scope, let's work directly on the auto-generated settings.
public class Settings
{
private Properties.Settings settings;
Settings(Properties.Settings settings)
{
this.settings = settings ?? throw new ArgumentNullException(nameof(settings));
}
// ..
}
We could then write cleaner code with much less redudancy.
public int WorkMinutes
{
get => settings.WorkMinutes;
set
{
settings.WorkMinutes = value;
Save();
}
}
public int RestMinutes
{
get => settings.RestMinutes;
set
{
settings.RestMinutes = value;
Save();
}
}
You no longer have a static class, but perhaps you would like a shared instance available throughout your application. Let's forsee a shared instance.
private static Settings shared;
public static Settings Default => shared ?? (shared = new Settings(Properties.Settings.Default));
Perhaps we could auto-initialise our settings in the constructor.
Settings(Properties.Settings settings)
{
this.settings = settings ?? throw new ArgumentNullException(nameof(settings));
Initialise();
}
Extended Design
In a real application, it's likely Settings
would end containing dozens of seperate settings properties. Perhaps the settings would also be editable at runtime. In addition, Initialise
would have to call Save
on each individual property. We could use a batch update on demand to optimize our class.
Initialise
would look like this:
public void Initialise()
{
using (var batch = new Batch(this))
{
if (WorkMinutes == 0)
{
WorkMinutes = 5;
}
if (RestMinutes == 0)
{
RestMinutes = 5;
}
// other settings ..
} // <- this calls Save() once
}
The pattern can be implemented as follows:
private void Save()
{
if (!deferred)
{
settings.Save();
}
}
private bool deferred;
public IDisposable StartBatch()
{
return new Batch(this);
}
private class Batch : IDisposable
{
Settings source;
public Batch(Settings source)
{
Debug.Assert(source != null);
this.source = source;
this.source.deferred = true;
}
public void Dispose()
{
source.deferred = false;
source.Save();
}
}
struct
makes little sense, a value type is a value, not a façade for some static object; there is no immutable data to encapsulate here - IMO this should be aclass
, if it needs to exist at all. \$\endgroup\$ – Mathieu Guindon♦ Oct 23 '16 at 21:16