It’s either puke worthy or elegant

protected static T redirectTo<T, CONTROLLER>(Expression<Func<CONTROLLER, object>> expression) where T : ViewModel, new()
{
return new T() {Override = new ControllerRedirectResult<CONTROLLER>(expression)};
}

Just a wee bit of code we’re using in our MVC Controller base class.  I’m happy with what it *does,* but tell me that isn’t butt ugly code.  I think the angle bracket and generic constraint noise tax is like the governor on school buses that keeps them from going over 65 miles an hour.  Anders’ way of telling you that you’re going too far and time to back off.  Anyway, the usage of the stuff above isn’t that bad, and the unit testing is pretty easy.

Comments are welcome.  This kind of thing bothers me in the sense that later developers are gonna scream WTF! when they find it in the code.
 

 

About Jeremy Miller

Jeremy is the Chief Software Architect at Dovetail Software, the coolest ISV in Austin. Jeremy began his IT career writing "Shadow IT" applications to automate his engineering documentation, then wandered into software development because it looked like more fun. Jeremy is the author of the open source StructureMap tool for Dependency Injection with .Net, StoryTeller for supercharged acceptance testing in .Net, and one of the principal developers behind FubuMVC. Jeremy's thoughts on all things software can be found at The Shade Tree Developer at http://codebetter.com/jeremymiller.
This entry was posted in Uncategorized. Bookmark the permalink. Follow any comments here with the RSS feed for this post.
  • http://www.key-logger.ws/ keylogger

    This Is Very interesting .. thanks for sharing this post

  • Brian Chiasson

    This gives me an opportunity to take Atwood’s recent advice and learn something new as I have not done anything with generics and this code is frying my brain.

    http://www.codinghorror.com/blog/archives/001138.html

    On another note, I am surprised you are using a static method here, I have found that statics are pure headaches when doing any TFD.

  • PandaWood

    Well, my take is that if you’d used TController instead of CONTROLLER, it would have taken me a few seconds longer to way “WTF”. It’s up to you to decide how much that is worth.

  • http://chadmyers.lostechies.com Chad Myers

    @Roederick Jeremy’s point was that ALT.NET means not doing things just because everyone else in the .NET/MSFT-o-sphere is doing it (i.e. VSS/TFS). We’ve found Subversion takes away a lot of pain/worry/stress of source control. So wide-sweeping refactorings are encouraged if it’s the right thing to do whereas, in other shops I’ve been in, you were afraid to make big changes for some of the reasons mentioned earlier, lack of test coverage, lots of file manipulation if not using R# to make the refactoring automatic, etc, etc, etc.

    As far as my comment re: VB: It’s really a pragmatic thing: Why should we concern ourselves with CLS compliance and VB.NET, JScript.NET, or any other .NET language other than the one(s) we’re using?

    If you or anyone else has a good argument, I’m all ears and I’m willing to consider it. At this time, however, it would appear to be a waste of time with no tangible benefit.

    If we were releasing a public API or an OSS project that might be used from VB.NET or some other .NET language, then absolutely, that would be an issue.

  • Roederick Sand

    @Colin

    “written media makes miscommunication likely.”

    Yep. What I actually ment was – ALT.NET doesn’t mean to be different for the difference sake.

    And I am taking A-word back.

  • http://colinjack.blogspot.com Colin Jack

    Sorry for 3 comments in a row but after a bit of thought my last comment was utter rubbish, ignore it! :)

  • http://colinjack.blogspot.com Colin Jack

    “Shift-F6 and rename “foo” to “Foo”
    Not so hard.”

    It can be a pain though, especially as for a large codebase you might well have multiple solutions (especially if multiple applications share some parts of the codebase).

  • http://colinjack.blogspot.com Colin Jack

    @Roederick
    I think that any issues you have with the tone are just down to the fact that written media makes miscommunication likely.

    Actually when Jeremy said he was being obnoxious he plainly wasn’t, and even re-reading his comments again I don’t see any reason you could possible take offense form them.

  • http://bugsquash.blogspot.com Mauricio

    Funny, I’ve been experimenting with a similar syntax myself for some time (although for Monorail instead of asp.net mvc), like:

    this.RedirectToAction(c => c.SomeAction);

    implemented as extension methods.

  • Roederick Sand

    @Jeremy

    I have no problems with your coding standards, I have problems with the tone of your responses to humble comments on this blog.

  • http://codebetter.com/blogs/jeremy.miller Jeremy D. Miller

    @ Roederick,

    Are you seriously calling me names and using BileBlog nomenclature just because I don’t care for the Microsoft coding standards, and instead, choose to use a *very* common standard (camel casing private and protected members) in .Net projects?

  • Roederick Sand

    @Jeremy
    Again, to be obnoxious. ALT.NET means …

    ALT.NET means a lot of things none of them being an ‘a**h****” I don’t know why, but after you (and Chad) have been kicked out of your last job, it’s no more fun to watch your attitude.

  • http://codebetter.com/blogs/jeremy.miller Jeremy D. Miller

    @Richard,

    Again, no it’s not. If you’re using a pessimistic locking approach for your source control, I guess it would be a PITA. With Subversion, I wouldn’t hesitate.

    Again, to be obnoxious. ALT.NET means I get to use Subversion and not have some of those problems.

  • http://chadmyers.lostechies.com Chad Myers

    @Frans it’s the static reflection technique that Daniel Cazzulino came up with for Moq:

    http://weblogs.asp.net/cazzu/archive/2006/07/06/Linq-beyond-queries_3A00_-strong_2D00_typed-reflection_2100_.aspx)

    Can you abuse it? Sure. Pass nulls into things can cause problems too, so I’m not sure why this would be any worse than anything else.

  • Richard

    I didn’t say it was hard. But it is a mess in version control.

  • http://codebetter.com/blogs/jeremy.miller Jeremy D. Miller

    @Richard,

    Shift-F6 and rename “foo” to “Foo”

    Not so hard.

  • Richard

    If protected method foo() is used in 100 places, and you decide to make it public, you have to change it to Foo() in 100 places.

  • http://weblogs.asp.net/fbouma FransBouma

    Jeremy: ok, though it could be done with just Func statements as well, then. The point with having Expression> is that someone could abuse the API without the ability to check whether it is abused (as the compiler can’t verify it).

    Indeed, using Funcs to access properties etc. is a neat way to write generic code without reflection. :)

  • http://chadmyers.lostechies.com Chad Myers

    @Mike: We’re writing an internal app and so we don’t really care about CLS compliance and certainly not whether it’s VB compatible. We’re primarily concerned with what helps us develop faster and subtle visual cues like underscore-prefixed member variables and camelCase’d protected/private members help us scan the code faster.

    RE: VIEWMODEL – This isn’t English, it’s C#, and if it makes the C# easier to read, that’s most important. We don’t have editors and proofreader’s scanning our code for English correctness.

    VIEWMODEL sticks out like a sore thumb, as it should. We have found it extremely disorienting to look into the middle of heavily-generic’d code and so every little visual cue that helps separate real code from generic code count.s

  • http://codebetter.com/blogs/jeremy.miller Jeremy D. Miller

    @Frans,

    We’re not manipulating the Expression in any way. Just using it as a way to pass a property name or method name. It’s effectively just compiler safe reflection, if you’ll let me get away with saying that. I’d call it a hacky way to do Ruby symbols.

    @Erik,

    There wouldn’t likely be a pascalCase and a camelCase method with the same name.

  • Mike Strobel

    @Jeremy: The problem with using camelCase for protected members is that those members are visible outside of your assembly. You are therefore polluting the consistency that has been carefully established across the .NET platform, not just in the base class library, but also in the vast majority of third-party frameworks. At the risk of sounding pompus, this is simply inconsiderate. It’s not just about what Microsoft says in its naming guidelines–it’s about what has been clearly and widely accepted by your .NET developer peers. This isn’t Java, and appeasing migrating Java developers does not, in my opinion, justify breaking from what is a clearly established standard for the platform. I used to be a hardcore Java programmer, but when I migrated to .NET, I completly changed my coding style, because the established Java styles were simply not appropriate. Your argument about PascalCase resembling VB code sounds flimsy as well–.NET is not a language; it’s a language-agnostic platform. As others have already been pointed out, there are some inconsistencies regarding uniqueness of named identifiers across programming language boundaries.

    @Chad: I have to disagree with your argument that generic type parameters should be named in all caps, and for much more fundamental reasons. I cannot think of any reason why a word, no less a sequence of words, should appear in all caps. VIEWMODEL is not an acronym. Moreover, one must visually scan the entire identifier (with more care than usual) because there is no clear separation between the individual words (in this case “view” and “model”). At least when using PascalCase, one can quickly distinguish the individual words in the identifier, and thus quickly establish its meaning. I see no compelling reason why generics type parameters should be made to stand out any more than they already do–between the presence of angle brackets and an appropriate prefix (e.g. the ‘T’ in “TViewModel”), any seasoned .NET/C++/Java developer should be able to instantly pick out a generic type parameter. I think, however, that the most compelling argument against using all caps is a simple consideration of aesthetics. THERE IS NO REASON WHY THIS SENTENCE NEEDS TO STICK OUT OF THIS PARAGRAPH LIKE A SORE THUMB. If you were to tell me that the preceding sentence was loud, obnoxious, and totally unnecessary, I would agree. If you really want your generic type parameters to stick out, consider writing a simple VS plugin to change the color, font face, or font weight of those parameters. Better yet, put in a feature request to the VS team to make generic type parameters a distinct element type in the ‘Fonts and Colors’ configuration.

    I do apologize if I sound like a pompus ass here. I realize that you don’t know me, and you have absolutely no reason to listen to me. However, the sheer number of compliments that I have received about my coding style suggests that I might just be on to something :).

  • http://neilmosafi.blogspot.com Neil Mosafi

    @Erik – correct. The spec states “For two identifiers to be considered distinct, they must differ by more than just their case.”. I personally use Pascal case for all methods and properties irrelvant of their access modifiers (though I initially hated doing so coming from a C++ background).

    Actually I gave it a go and both methods would actually be inaccessible – in VB you receive the build error ‘Foo’ is ambiguous because multiple kinds of members with this name exist in class Class1

  • http://weblogs.asp.net/fbouma FransBouma

    What I wonder about is why there’s an expression tree passed in instead of a Func. The expression tree suggests that the expression is modified along the code path with more expressions being added to it, and I assume it’s then compiled inside the controller to a delegate.

    This IMHO is a bad thing to do, as it is a way of self modifying code. IMHO, in-memory code shouldn’t use expression trees but simply Func statements which are verifyable at compile time and you can determine what’s passed in in a reasonable way.

  • http://colinjack.blogspot.com Colin Jack

    Couldn’t agree more. We have a few ultra useful classes deep in our domain model that use generics a lot including to multiple levels. It’s a nighmare to read that code especially when the generics is on the method.

    I think when we discussed our issues we decided that perhaps we were trying to do too much in the base class in a typesafe manner. Very DRY but very un-readable. We might have been better just using composition and each composed class would have had less generic arguments. Didn’t take it any further though because we aren’t working on that part of the code right now.

    Also could you not give T a better name, it would make the line longer but might help slightly.

  • Erik

    @Jeremy – I think Neil’s point is that because VB.Net is case insensitive, ‘Foo’ == ‘foo’ for that language. Therefore, one of these methods would be inaccessible.

  • http://codebetter.com/blogs/jeremy.miller Jeremy D. Miller

    @Neil,

    Um Neil, Pascal case or Camel casing has absolutely nothing to do with whether or not my assembly could be used by VB.Net code.

  • Mike Suarez

    yep i find myself tangled in generics syntax a lot nowadays n’ i share the feeling … it makes sense but adds some verbosity n’ ugliness … we might be evolving into something :) i hope it turns prettier soon ..

  • http://chadmyers.lostechies.com Chad Myers

    @Bill et al
    I was skeptical about the GENERICTYPE vs. TGenericType stuff at first, but as we started doing more generics it became clear that one important rule about doing generics is that you REALLY want the generic type to stand out as much as possible.

    I’m sorry, but TViewModel blends in too easily with the rest of the code whereas VIEWMODEL stands out like a sore thumb (which is the desired effect.

  • http://neilmosafi.blogspot.com Neil Mosafi

    Not sure about that. If you have that coding standard then it’s perfectly valid for someone to do this:

    public void Foo()
    protected void foo()

    This would of course (if you care) render your assembly non CLS compliant and would mean all those poor VB.NET developers out there would not be able to use it!

  • http://codebetter.com/blogs/jeremy.miller Jeremy D. Miller

    @All,

    I’m not wild about the TWhatever, MS guideline or not. We use all caps in our standard and we’re consistent.
    The only really important thing about a coding standard is consistency anyway.

    @Mike,

    *Our* coding standard says camel casing for protected and private methods. Pascal casing everywhere makes your code look way too much like VB. My ideas about what a coding standard should be were formed on projects with Java guys earlier in the decade.

    The awesome thing about being ALT.NET is being able to disregard MS coding standards if you don’t like them;-)

  • Mike Strobel

    I second Bill. WTF is up with the “CONTROLLER” generic constraint? It should be “TController”, and your method name should be “RedirectTo” rather than “redirectTo”. PascalCase is explicitly recommended in Microsoft’s naming guidelines for non-private members.

  • http://neilmosafi.blogspot.com Neil Mosafi

    Yeah, it’s C#, it’s strongly typed, and that’s just the way it is I guess! A junior developer would probably not be comfy with it, but hopefully it would be hidden in the lower depths of an API and hot need to be changed EVER.

    Regarding the naming, I would go with the T prefix such as:

    protected static TViewModel RedirectTo(Expression> expression)
    where TViewModel : ViewModel, new()

    Perhaps some better aliasing support in the compiler would be nice, something like

    using FuncObjectExpression
    = Expression>;

    Which would then make the function

    RedirectTo< ..>(FuncObjectExpression expression)

  • Bill

    Well, CONTROLLER should be TController, but other than that I don’t see any problem with it.