I was thinking of sending Phil an email with this suggestion.. but as it hasn't been looked over from all angles (didn't use it in real code... just thinking out loud because of some testability issues I am having) didn't do it and had an idea to post it
here to see what others think.
Ok... first to say that currently it's written as inherited from the default Controller, but my suggestion would actually be that this IS how the default controller looks (or better said, take a look at the bits and imagine that the rest of the Controller
code is there :) and that this is not an inherited implementation of Controller, but rather the Controller itself)
A trully generic controller:
public class TestableController<TViewData, TTempData> : Controller {
public string ViewName
{ get
{ return _viewName;
}
} private string _viewName;
public string MasterName
{ get
{ return _masterName;
}
} private string _masterName;
public TViewData ViewData { get { return _viewData;
}
} private TViewData _viewData;
public TTempData TempData
{ get
{ return _tempData;
}
} private TTempData _tempData;
public virtual void RenderView(string viewName)
{
_viewName = viewName;
What problem would the generic renderview be addressing? You already have the capability to send typed data to the view. I'm just curious, perhaps there's something I'm overlooking.
- The current controller is not type-safe - although your view is <SomeType> for ViewData in your controller you can basically *suff* anything in the ViewData as it accepts an object - it suffers from all problems we've dealt with generics (and it's not
clear for the developers what they should send down the pipe to view)
- When I was trying to test the controller and the data sent down the pipe (ViewData) I had to both check if it had the typed data or if it had that data in the dictionary - this is not intention revealing, this is not clear, this is not good, it has too
much noise... not good... You should be able to say:
List<Order> orders = controller.ViewData as List<Order>
And that's it... this should be un-ambigious - especially now that we have the means (= Generics)
- Why like this... because currently to get the Rendered View, ViewData and Master in a test you need to subclass that controller... yuck... with this approach you could upon RenderView save it and extract it through the properties (which don't exist for
ViewName and MasterName) and again... get the type-safed ViewData which was sent down the pipe
That's why there is a non generic version of the Controller - which inherits the generic one, only implements it with the dictionaries - that is the correct way to handle this.
But then - as with typed way - the intent is clear
Ok... I've found why this is so (actually not sure how I didn't figure this out before). Each action can send it's own type of ViewData, and one can use specific ViewData, other doesn't need to... That's why it was made both ways.
But still... this suffers from all the problems above... wonder what MS guys think? Any plans for the next CTP which will adress these concerns a bit (I already know that RenderView will be made public for testing purposes)?
Vladan Strig...
Participant
1011 Points
222 Posts
What do you think, should default controllers look like this?
Jan 06, 2008 11:42 PM|LINK
I was thinking of sending Phil an email with this suggestion.. but as it hasn't been looked over from all angles (didn't use it in real code... just thinking out loud because of some testability issues I am having) didn't do it and had an idea to post it here to see what others think.
Ok... first to say that currently it's written as inherited from the default Controller, but my suggestion would actually be that this IS how the default controller looks (or better said, take a look at the bits and imagine that the rest of the Controller code is there :) and that this is not an inherited implementation of Controller, but rather the Controller itself)
A trully generic controller:
public class TestableController<TViewData, TTempData> : Controller {
public string ViewName
{
get
{
return _viewName;
}
}
private string _viewName;
public string MasterName
{
get
{
return _masterName;
}
}
private string _masterName;
public TViewData ViewData {
get {
return _viewData;
}
}
private TViewData _viewData;
public TTempData TempData
{
get
{
return _tempData;
}
}
private TTempData _tempData;
public virtual void RenderView(string viewName)
{
_viewName = viewName;
base.RenderView(viewName);
}
public virtual void RenderView(string viewName, TViewData viewData)
{
_viewName = viewName;
_viewData = viewData;
base.RenderView(viewName, viewData);
}
public virtual void RenderView(string viewName, string masterName, TViewData viewData)
{
_viewName = viewName;
_masterName = masterName;
_viewData = viewData;
base.RenderView(viewName, masterName, viewData);
}
}
And the default generics-less implementation (to support the dictionary scenario which you can use now which is ok):
public class TestableController : TestableController<Dictionary<string, object>, Dictionary<string, object>>
{
}
What do you think, is this worthwhile suggesting or?
CVertex
Member
147 Points
128 Posts
Re: What do you think, should default controllers look like this?
Jan 07, 2008 04:41 AM|LINK
I like the TTempData type parameter idea, but I think the view should be specifying the view data type, not the controller.
It would be nice to have a generic RenderView though.
Something like RenderView<TViewData>(string viewName, TViewData data);
ChadThiele
Participant
983 Points
274 Posts
Re: What do you think, should default controllers look like this?
Jan 07, 2008 05:08 AM|LINK
What problem would the generic renderview be addressing? You already have the capability to send typed data to the view. I'm just curious, perhaps there's something I'm overlooking.
Thanks!
CVertex
Member
147 Points
128 Posts
Re: What do you think, should default controllers look like this?
Jan 07, 2008 06:34 AM|LINK
None I guess, and it would complicate subclasses that override RenderView too.
Vladan Strig...
Participant
1011 Points
222 Posts
Re: What do you think, should default controllers look like this?
Jan 07, 2008 07:16 AM|LINK
Actually it's a both ways thing... if you want to be type-safe it needs to be specified on both view and controller.
CVertex
Member
147 Points
128 Posts
Re: What do you think, should default controllers look like this?
Jan 07, 2008 07:22 AM|LINK
For me, it doesn't seem it's all that common to use the same TViewData for all RenderView() calls.
Think of a basic CRUD scenario
Vladan Strig...
Participant
1011 Points
222 Posts
Re: What do you think, should default controllers look like this?
Jan 07, 2008 07:31 AM|LINK
Several things:
- The current controller is not type-safe - although your view is <SomeType> for ViewData in your controller you can basically *suff* anything in the ViewData as it accepts an object - it suffers from all problems we've dealt with generics (and it's not clear for the developers what they should send down the pipe to view)
- When I was trying to test the controller and the data sent down the pipe (ViewData) I had to both check if it had the typed data or if it had that data in the dictionary - this is not intention revealing, this is not clear, this is not good, it has too much noise... not good... You should be able to say:
List<Order> orders = controller.ViewData as List<Order>
And that's it... this should be un-ambigious - especially now that we have the means (= Generics)
- Why like this... because currently to get the Rendered View, ViewData and Master in a test you need to subclass that controller... yuck... with this approach you could upon RenderView save it and extract it through the properties (which don't exist for ViewName and MasterName) and again... get the type-safed ViewData which was sent down the pipe
Vladan Strig...
Participant
1011 Points
222 Posts
Re: What do you think, should default controllers look like this?
Jan 07, 2008 07:38 AM|LINK
That's why there is a non generic version of the Controller - which inherits the generic one, only implements it with the dictionaries - that is the correct way to handle this.
But then - as with typed way - the intent is clear
Vladan Strig...
Participant
1011 Points
222 Posts
Re: What do you think, should default controllers look like this?
Jan 07, 2008 08:03 AM|LINK
Ok... I've found why this is so (actually not sure how I didn't figure this out before). Each action can send it's own type of ViewData, and one can use specific ViewData, other doesn't need to... That's why it was made both ways.
But still... this suffers from all the problems above... wonder what MS guys think? Any plans for the next CTP which will adress these concerns a bit (I already know that RenderView will be made public for testing purposes)?