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

Displaying different tutorials about OneNote

Submitted by: @import:stackexchange-codereview··
0
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 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.