patterncsharpMinor
TabController for handling opening and closing of tabs
Viewed 0 times
handlingforandopeningclosingtabcontrollertabs
Problem
I created a class
```
public abstract class TabController
{
ViewModel vm;
#region constructor
public TabController(ViewModel vm)
{
this.vm = vm;
}
#endregion
public abstract void go(Uri address);
public abstract void newTabGo(Uri address);
public abstract void newEmptyTab();
public void newTab(Uri address, int insertionIndex)
{
vm.browserCollection.Insert(insertionIndex, new MyBrowserControl(address));
}
}
//first class, starts out this way when the program starts -- no tabs are open
public class NoTabsOpen : TabController
{
ViewModel vm;
#region constructor
public NoTabsOpen(ViewModel vm)
: base(vm)
{
this.vm = vm;
}
#endregion
public override void go(Uri address)
{
if (address != null)
vm.browserCollection.Add(new MyBrowserControl(address));
}
public override void newTabGo(Uri address)
{
if (address != null)
base.newTab(address, 0);
}
public override void newEmptyTab()
{
base.newTab(null, 0);
}
}
//second class - active when at least one tab is open
public class TabsOpen : TabController
{
ViewModel vm;
#region constructor
public TabsOpen(ViewModel vm)
: base(vm)
{
this.vm = vm;
}
#endregion
public override void go(Uri address)
{
if (address != null)
{
MyBrowserControl mbc = vm.browserCollection[vm.activeWindow];
mbc.mySource = address;
}
}
public override void n
TabController to handle opening and closing of tabs. It needs some data from my viewModel, so should I pass in the viewmodel via the constructor or is that overkill? It really only needs the parameters browserCollection and activeWindow, so I could just pass those two in if that's better. I'm just starting learning about design patterns so I can code well but am having trouble wiring everything up.```
public abstract class TabController
{
ViewModel vm;
#region constructor
public TabController(ViewModel vm)
{
this.vm = vm;
}
#endregion
public abstract void go(Uri address);
public abstract void newTabGo(Uri address);
public abstract void newEmptyTab();
public void newTab(Uri address, int insertionIndex)
{
vm.browserCollection.Insert(insertionIndex, new MyBrowserControl(address));
}
}
//first class, starts out this way when the program starts -- no tabs are open
public class NoTabsOpen : TabController
{
ViewModel vm;
#region constructor
public NoTabsOpen(ViewModel vm)
: base(vm)
{
this.vm = vm;
}
#endregion
public override void go(Uri address)
{
if (address != null)
vm.browserCollection.Add(new MyBrowserControl(address));
}
public override void newTabGo(Uri address)
{
if (address != null)
base.newTab(address, 0);
}
public override void newEmptyTab()
{
base.newTab(null, 0);
}
}
//second class - active when at least one tab is open
public class TabsOpen : TabController
{
ViewModel vm;
#region constructor
public TabsOpen(ViewModel vm)
: base(vm)
{
this.vm = vm;
}
#endregion
public override void go(Uri address)
{
if (address != null)
{
MyBrowserControl mbc = vm.browserCollection[vm.activeWindow];
mbc.mySource = address;
}
}
public override void n
Solution
public TabsOpen(ViewModel vm)
: base(vm)
{
this.vm = vm;
}There's no reason to set
this.vm = vm here. You're already doing that by passing the argument to your base class' constructor. Both of your subclass ctors could look like this. public TabsOpen(ViewModel viewModel)
: base (viewModel) { }I changed the parameter name to make it a bit clearer. Speaking of naming, it's a convention in C# to PascalCase method names. You've camel cased yours. It's consistent though, so I can't gripe too much about it.
To address your actual question though, typically yes, you will pass your view model to the controller as you've done here. However, that's because the controller is typically updating the view. That doesn't seem to be the case here, so the thought comes to mind that it is better to only inject what you actually need.
I like that you've used polymorphism to simulate the state of an object, but it makes me wonder if what you have here is really a view model and not some other concept.
Code Snippets
public TabsOpen(ViewModel vm)
: base(vm)
{
this.vm = vm;
}public TabsOpen(ViewModel viewModel)
: base (viewModel) { }Context
StackExchange Code Review Q#83027, answer score: 3
Revisions (0)
No revisions yet.