HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Handle Settings in Windows App

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
settingsappwindowshandle

Problem

So, I fixed my problem with a public ViewModel for my MainPage like this:

MainPage.xaml.cs:

private static MainPageVM Data = new MainPageVM();
public MainPage()
{
this.InitializeComponent();
SettingsPane.GetForCurrentView().CommandsRequested += OnCommandsRequested;

this.DataContext = Data;

this.SizeChanged += (s, args) =>
{
PageSizeChanged();
};

Windows.Storage.ApplicationData.Current.DataChanged += (a, o) =>
{
CoreApplication.MainView.CoreWindow.Dispatcher.RunAsync(CoreDispatcherPriority.Normal, () =>
{
Data.Theme = (int)ApplicationData.Current.RoamingSettings.Values["Theme"];
});
};
}


MainPageVM.cs:

private void SetValues()
{
if (Windows.Storage.ApplicationData.Current.RoamingSettings.Values.ContainsKey("Theme"))
{
int value = (int)Windows.Storage.ApplicationData.Current.RoamingSettings.Values["Theme"];
if (value >= 0 && value

Settings.xaml.cs:

private void InitSettings()
{
if (Windows.Storage.ApplicationData.Current.RoamingSettings.Values.ContainsKey("Theme"))
{
int value = (int)Windows.Storage.ApplicationData.Current.RoamingSettings.Values["Theme"];
if (value >= 0 && value

Settings.xaml:


Text and background color:









There is similar code for each setting, but this is a good representation of it. It doesn't feel right to use an int to represent my theme; should this be an enum? Please tell me all the problems now so I don't have to refactor again later.

Solution

I don't really like the dependencies on all that static stuff. It effectively means your code becomes very hard to test.

Also this code:

if (Windows.Storage.ApplicationData.Current.RoamingSettings.Values.ContainsKey("Theme"))
{
    int value = (int)Windows.Storage.ApplicationData.Current.RoamingSettings.Values["Theme"];
    if (value >= 0 && value <= 2) { Theme = value; }
    else { Theme = 0; }
}
else { Theme = 0; }


seems repeated and should be consolidated into a single method.

I'd start cleaning it up by introducing an IThemeProvider which can be injected into your MainPage, MainPageVM and Settings classes. Would probably look like this:

class ThemeChangedEventArgs : EventArgs
{
    public readonly int NewTheme;

    public ThemeChangedEventArgs(int newTheme)
    {
        NewTheme = newTheme;
    }
}

interface IThemeProvider
{
    int GetCurrentTheme();
    void SetCurrentTheme(int theme);
    event EventHandler ThemeChanged;
}


Then your MainPage would look something like this:

private MainPageVM Data = new MainPageVM();
private IThemeProvider _ThemeProvider;

public MainPage(IThemeProvider themeProvider)
{
    _ThemeProvider = themeProvider;

    this.InitializeComponent();
    SettingsPane.GetForCurrentView().CommandsRequested += OnCommandsRequested;

    this.DataContext = Data;

    this.SizeChanged += (s, args) => PageSizeChanged()
    _ThemeProvider.ThemeChanged += (s, e) => Data.Theme = e.NewTheme;
}


MainPageVM:

private IThemeProvider _ThemeProvider; 
public MainPageVM(IThemeProvider themeProvider)
{
    _ThemeProvider = themeProvider;
}

private void SetValues()
{
    Theme = _ThemeProvider.GetCurrentTheme();
}


and Settings:

private IThemeProvider _ThemeProvider; 
public Settings(IThemeProvider themeProvider)
{
    _ThemeProvider = themeProvider;
}

private void InitSettings()
{
    Theme.SelectedIndex = _ThemeProvider.GetCurrentTheme();
}

private void ThemeChanged(object sender, SelectionChangedEventArgs e)
{
    _ThemeProvider.SetCurrentTheme(Theme.SelectedIndex);
}


Code looks a lot cleaner now I'd say and you can easily mock the IThemeProvider out and test it.

Implementation of the provider could be like this (based on what you currently do):

public class ApplicationDataThemeProvider : IThemeProvider
{
    public ApplicationDataThemeProvider()
    {
        Windows.Storage.ApplicationData.Current.DataChanged += (a, o) =>
        {
            CoreApplication.MainView.CoreWindow.Dispatcher.RunAsync(CoreDispatcherPriority.Normal, () =>
            {
                OnThemeChanged((int)ApplicationData.Current.RoamingSettings.Values["Theme"]);
            });
        };
    }

    public int GetCurrentTheme()
    {
        if (Windows.Storage.ApplicationData.Current.RoamingSettings.Values.ContainsKey("Theme"))
        {
            int value = (int)Windows.Storage.ApplicationData.Current.RoamingSettings.Values["Theme"];
            return (value >= 0 && value  ThemeChanged;
    private void OnThemeChanged(int newTheme)
    {
        var handler = ThemeChanged;
        if (handler != null)
        {
            handler(this, new ThemeChangedEventArgs(newTheme));
        }
    }
}


This encapsulates all the nasty global app data stuff in a single class where it should be a lot easier to handle and debug.

Code Snippets

if (Windows.Storage.ApplicationData.Current.RoamingSettings.Values.ContainsKey("Theme"))
{
    int value = (int)Windows.Storage.ApplicationData.Current.RoamingSettings.Values["Theme"];
    if (value >= 0 && value <= 2) { Theme = value; }
    else { Theme = 0; }
}
else { Theme = 0; }
class ThemeChangedEventArgs : EventArgs
{
    public readonly int NewTheme;

    public ThemeChangedEventArgs(int newTheme)
    {
        NewTheme = newTheme;
    }
}

interface IThemeProvider
{
    int GetCurrentTheme();
    void SetCurrentTheme(int theme);
    event EventHandler<ThemeChangedEventArgs> ThemeChanged;
}
private MainPageVM Data = new MainPageVM();
private IThemeProvider _ThemeProvider;

public MainPage(IThemeProvider themeProvider)
{
    _ThemeProvider = themeProvider;

    this.InitializeComponent();
    SettingsPane.GetForCurrentView().CommandsRequested += OnCommandsRequested;

    this.DataContext = Data;

    this.SizeChanged += (s, args) => PageSizeChanged()
    _ThemeProvider.ThemeChanged += (s, e) => Data.Theme = e.NewTheme;
}
private IThemeProvider _ThemeProvider; 
public MainPageVM(IThemeProvider themeProvider)
{
    _ThemeProvider = themeProvider;
}

private void SetValues()
{
    Theme = _ThemeProvider.GetCurrentTheme();
}
private IThemeProvider _ThemeProvider; 
public Settings(IThemeProvider themeProvider)
{
    _ThemeProvider = themeProvider;
}

private void InitSettings()
{
    Theme.SelectedIndex = _ThemeProvider.GetCurrentTheme();
}

private void ThemeChanged(object sender, SelectionChangedEventArgs e)
{
    _ThemeProvider.SetCurrentTheme(Theme.SelectedIndex);
}

Context

StackExchange Code Review Q#77850, answer score: 4

Revisions (0)

No revisions yet.