patterncsharpMinor
Displaying different tutorials about OneNote
Viewed 0 times
tutorialsonenotedisplayingdifferentabout
Problem
In my app, I have this code to display different tutorials about OneNote. This clears and resets certain values, and updates some variables so we know which version we are working with. The main problem is it has a lot of repetition in the three
```
private void SwitchVersion(string version)
{
// This programatically selects the top value that is always the same
// to prevent crashes. Items is a ListBox.
Items.SelectedValue = "OneNote";
// This clears all values except the top value.
// Data.ItemList is an ObservableCollection bound to Items.
while (Data.ItemList.Count > 1)
{
Data.ItemList.RemoveAt(1);
}
// This clears the ObservableCollection of XAML pages navigated to with
// the ListBox. These values are tied to the values in ItemList.
Data.Pages.Clear();
if (version == "Phone")
{
foreach (string s in GlobalVars.PHome())
{
if (s == "OneNote")
{
continue;
}
Data.ItemList.Add(s);
}
foreach (Type t in GlobalVars.PHomeP())
{
Data.Pages.Add(t);
}
// This is the method called when a value in the ListBox is chosen.
// We need to call this to update some submenus.
Data.NewSelectionP("OneNote");
}
if(version == "WStore")
{
foreach (string s in GlobalVars.Home())
{
if (s == "OneNote") continue;
Data.ItemList.Add(s);
}
foreach (Type t in GlobalVars.HomeP()) Data.Pages.Add(t);
Data.NewSelectionWS("OneNote");
}
if (version == "2013")
{
foreach (string s in GlobalVars.Home2013())
{
if (s == "OneNote") continue;
Data.ItemList.Add(s);
}
foreach (Type t in GlobalVars.HomeP2013()) Data.Pages.Add(t);
Data.NewSelection2013("OneNote");
}
// This is the Frame used t
ifs, although all comments are welcomed.```
private void SwitchVersion(string version)
{
// This programatically selects the top value that is always the same
// to prevent crashes. Items is a ListBox.
Items.SelectedValue = "OneNote";
// This clears all values except the top value.
// Data.ItemList is an ObservableCollection bound to Items.
while (Data.ItemList.Count > 1)
{
Data.ItemList.RemoveAt(1);
}
// This clears the ObservableCollection of XAML pages navigated to with
// the ListBox. These values are tied to the values in ItemList.
Data.Pages.Clear();
if (version == "Phone")
{
foreach (string s in GlobalVars.PHome())
{
if (s == "OneNote")
{
continue;
}
Data.ItemList.Add(s);
}
foreach (Type t in GlobalVars.PHomeP())
{
Data.Pages.Add(t);
}
// This is the method called when a value in the ListBox is chosen.
// We need to call this to update some submenus.
Data.NewSelectionP("OneNote");
}
if(version == "WStore")
{
foreach (string s in GlobalVars.Home())
{
if (s == "OneNote") continue;
Data.ItemList.Add(s);
}
foreach (Type t in GlobalVars.HomeP()) Data.Pages.Add(t);
Data.NewSelectionWS("OneNote");
}
if (version == "2013")
{
foreach (string s in GlobalVars.Home2013())
{
if (s == "OneNote") continue;
Data.ItemList.Add(s);
}
foreach (Type t in GlobalVars.HomeP2013()) Data.Pages.Add(t);
Data.NewSelection2013("OneNote");
}
// This is the Frame used t
Solution
while (Data.ItemList.Count > 1)
{
Data.ItemList.RemoveAt(1);
}I'm not exactly sure what
ItemList is, but I would implement a Clear method to do this. If it's not your code, write an extension method.As for your actual question, it's very similar to the switch smell and could probably be solved the same way.
if (version == "Phone")
if(version == "WStore")
if (version == "2013")Sometimes you're taking an identical action, sometimes your not. So, you could definitely start with extracting the duplicated logic out into a private method, but I would also consider a better OOP approach.
I don't really have enough context to show you how I would go about that, but the general gist is you should have a base class that implements the "default" behavior of your class. Then, you would implement a child class for each each of these different
versions. The child class would override the appropriate behavior.Code Snippets
while (Data.ItemList.Count > 1)
{
Data.ItemList.RemoveAt(1);
}if (version == "Phone")
if(version == "WStore")
if (version == "2013")Context
StackExchange Code Review Q#74798, answer score: 2
Revisions (0)
No revisions yet.