In a MVVM application (Xamarin.Forms, FWIW) I have a viewmodel that stores settings explicitly
public class SettingsPageViewMode : INavigatedAware
{
ISettingsRepository settingsRepository; // injected
public void OnNavigatedTo(NavigationParameters navigationParameters)
{
var settings = settingsRepository.LoadSettings();
this.SomeBoolSetting = settings.SomeBoolSetting;
}
public bool SomeBoolSetting
{
get => someBoolSetting;
set
{
if(someBoolSetting == value)
{
return;
}
someBoolSetting = value;
OnPropertyChanged("SomeBoolSetting");
UpdateSettings();
}
}
private void UpdateSettings()
{
var settings = settingsRepository.LoadSettings();
settings.SomeBoolSetting = SomeBoolSetting;
settingsRepository.SaveSettings(settings);
}
}
I am thinking about if it's a good idea to have settings store itself automatically via a decorator that is instantiated when ISettingsRepository
returns a Settings
object
internal class AutoPersistSettingsDecorator : Settings
{
Settings settings;
ISettingsRepository settingsRepository;
public AutoPersistSettingsDecorator(Settings settings, ISettingsRepository settingsRepository)
{
this.settings = settings;
this.settingsRepository = settingsRepository;
}
public override bool SomeBoolSetting
{
get => settings.SomeBoolSetting;
set
{
if(settings.SomeBoolSetting == value)
{
return;
}
settings.SomeBoolSetting = value;
settingsRepository.SaveSettings(settings);
}
}
}
This would simplify the viewmodel and make saving the settings transparent
public class SettingsPageViewMode : INavigatedAware
{
ISettingsRepository settingsRepository; // injected
Settings settings;
public void OnNavigatedTo(NavigationParameters navigationParameters)
{
var settings = settingsRepository.LoadSettings();
this.SomeBoolSetting = settings.SomeBoolSetting;
}
public bool SomeBoolSetting
{
get => settings.SomeBoolSetting;
set
{
if(settings.SomeBoolSetting == value)
{
return;
}
settings.SomeBoolSetting = value;
OnPropertyChanged("SomeBoolSetting");
}
}
}
Possible drawbacks:
Settings
looks like a data type, but comes with a behavior- It's not really obvious that
Settings
is persisted - Might violate the principle of least astonishment
- When the public interface of
Settings
changes,AutoPersistSettingsDecorator
has to change, too - Might be error prone
However, Clean Code still proposes (more or less) this design (I don't have a page number, since I'm reading on a Kindle, but it's in part 11 (Systems), in the sections about AOP, just after Figure 11-3)
The client believes it is invoking
getAccounts()
on aBank
object, but it is actually talking to a set of nested decorator objects that extend the basic behavior of theBank
POJO.
Did I get something fundamentally wrong? Should I create the settings class more use-case centred? How could I possibly achieve this, since I basically have a very data-like view of a settings class. Is this approach appropriate at all when it comes to settings?