TerryR my friend, you and I should have a drink. We have some similar problems.
1. Project Structure: I agree with Eduardo that the folder structure in an MVC app leaves something to be desired. You have your standard Controllers, Models, and Views folders. But then the Views folder gets broken down into a different folder for each Controller, plus a Shared folder. And each Views/ControllerName or Views/Shared can be broken down into EditorTemplates and DisplayTemplates. But it lets you decide how to organize your Models folder (you can do with or without subfolders & additional namespace declarations).
God forbid you are using Areas, which duplicate the Controllers, Models, and Views folder structure for each area.
/Areas
/Area1Name
/Controllers
FirstController.cs
SecondController.cs
ThirdController.cs
/Models
(can organize all in here or in separate folders / namespaces)
/Views
/First
/DisplayTemplates
WidgetAbc.cshtml <-- to be used by views in Views/First
/EditorTemplates
WidgetAbc.cshtml <-- to be used by views in Views/First
PartialViewAbc.cshtml <-- to be used by FirstController
/Second
PartialViewDef.cshtml <-- to be used by SecondController
/Third
PartialViewMno.cshtml <-- to be used by ThirdController
/Shared
/DisplayTemplates
WidgetXyz.cshtml <-- to be used by any view in Area1
/EditorTemplates
WidgetXyz.cshtml <-- to be used by any view in Area1
PartialViewXyz.cshtml <-- to be used anywhere in Area1
_ViewStart.cshtml <-- area needs its own _ViewStart.cshtml
Web.config <-- put custom HTML Helper namespaces in here
Area1NameRegistration.cs <-- define routes for area1 here
/Area2Name
/Controllers
/Models
/Views
Area2NameRegistration.cs <-- define routes for area2 here
/Controllers
AccountController.cs
HomeController.cs
/Models
/Views
/Account
/DisplayTemplates
WidgetGhi.cshtml <-- to be used views in Views/Account
/EditorTemplates
WidgetGhi.cshtml <-- to be used views in Views/Account
PartialViewGhi.cshtml <-- to be used by AccountController
/Home
(same pattern as Account, views & templates are controller-specific)
/Shared
/DisplayTemplates
EmailAddress.cshtml <-- to be used by any view in any area
Time.cshtml <-- to be used by any view in any area
Url.cshtml <-- to be used by any view in any area
/EditorTemplates
EmailAddress.cshtml <-- to be used by any view in any area
Time.cshtml <-- to be used by any view in any area
Url.cshtml <-- to be used by any view in any area
_Layout.cshtml <-- master layout page with sections
Error.cshtml <-- custom page to show if unhandled exception occurs
_ViewStart.cshtml <-- won't be used automatically in an area
Web.config <-- put custom HTML Helper namespaces in here
This means if you are working with something like a WidgetController, you have to look in other folders to find the related WidgetViewModels, WidgetViews, WidgetEditorTemplates, WidgetDisplayTemplates, etc. As cumbersome as this may be, I stick to it and don't deviate from these MVC conventions. As far as putting a model, controller, and view in the same folder but with different namespaces, I avoid this because I use ReSharper. It will squiggly underline a namespace that doesn't match the folder where the class is located. I know I could turn this R# feature off, but it helps in other parts of the project.
For non-class files, MVC gives you Content and Scripts out of the box. We try to keep all of our static / non-compiled files in these places, again, to follow convention. Any time we incorporate a js library that uses themes (images and or css), the theme files all go somewhere under /content. For script, we just put all of them directly into /scripts. Originally this was to get JS intellisense from VS, but now that we get JS intellisense from R# regardless of placement in /scripts, I suppose we could deviate from that and divide scripts by folder to better organize. Are you using ReSharper? It is pure gold IMO.
Another little piece of gold that helps a lot with refactoring is T4MVC. Using this, we don't need to type in string paths for area names, controller names, action names, even files in content & scripts. T4MVC strongly types all of the magic strings for you. Here's a small sample of how your project structure doesn't matter as much if you're using T4MVC:
// no more magic strings in route definitions
context.MapRoutes(null,
new[] { string.Empty, "features", "features/{version}" },
new
{
area = MVC.PreviewArea.Name,
controller = MVC.PreviewArea.Features.Name,
action = MVC.PreviewArea.Features.ActionNames.ForPreview,
version = "december-2011-preview-1",
},
new { httpMethod = new HttpMethodConstraint("GET") }
);
@* T4MVC renders .min.js script versions when project is targeted for release *@
<link href="@Url.Content(Links.content.Site_css)?r=201112B" rel="stylesheet" />
<script src="@Url.Content(Links.scripts.jquery_1_7_1_js)" type="text/javascript">
</script>
@* render a route URL as if you were calling an action method directly *@
<a href="@Url.Action(MVC.MyAreaName.MyControllerName.MyActionName
(Model.SomeId))">@Html.DisplayFor(m => m.SomeText)</a>
// call action redirects as if you were executing an action method
return RedirectToAction(MVC.Area.MyController.DoSomething(obj1.Prop, null));
2. Data access: I have no experience with PetaPoco, but I'm sure it's worth checking out. For your complex reports, have you considered SQL Server Reporting services? Or, are you running on a different db? Sorry I'm not clear on what exactly you are asking for. We use EF + LINQ, but we also put certain knowledge about how to generate reports in domain classes. Thus, we have controller call domain service call repository, instead of having controller call repository directly. For ad-hoc reports we use SQL Reporting Services, which again isn't perfect, but our users like to be able to bring data into Excel easily, and SSRS makes that easy on us.
3. Client-Side Code Organization and UI Rendering: This is where I think I may be able to offer some help. Take a page from the book of MVC unobtrusive validation and unobtrusive AJAX. Consider this:
<img id="loading_spinner" src="/path/to/img" style="display:none;" />
<h2 id="loading_results" style="display:none;">
Please wait, this may take a while...
</h2>
<div id="results">
</div>
<input id="doSomethingDangerous" class="u-std-ajax"
type="button" value="I'm feeling lucky"
data-myapp-confirm="Are you sure you want to do this?"
data-myapp-show="loading_spinner,loading_results"
data-myapp-href="blah/DoDangerousThing" />
Ignore the ajax success function for now (more on this later). You can get away with a single script for some of your actions:
$('.u-std-ajax').click(function () {
// maybe confirm something first
var clicked = this;
var confirmMessage = $(clicked).data('myapp-confirm');
if (confirmMessage && !confirm(confirmMessage )) { return; }
// show a spinner? something global would be preferred so
// I dont have to repeat this on every page
// maybe the page should notify the user of what's going on
// in addition to the dialog?
var show = $(clicked).data('myapp-show');
if (show) {
var i, showIds = show.split(',');
for (i = 0; i < showIds.length; i++) {
$('#' + showIds[i]).show();
}
}
var url = $(clicked).data('myapp-href');
if (url) {
$.ajax({
url: url,
complete: function () {
// Need to hide the spinner, again would prefer to
// have this done elsewhere
if (show) {
for (i = 0; i < showIds.length; i++) {
$('#' + showIds[i]).hide();
}
}
}
});
}
});
The above code will take care of the confirmation, showing the spinner, showing the wait message, and hiding the spinner / wait message after the ajax call is complete. You configure the behaviors using data-* attributes, like the unobtrusive libraries.
General Questions
- Client MVC vs. server MVC? I didn't try to librarify the actions you took in the success function because it looks like your controller is returning JSON. If your controllers are returning JSON, you might want to look at KnockoutJS. Knockout JS version 2.0 was released today. It can plug right into your JSON, so that an observable click can automatically bind data to your javascript templates. On the other hand if you don't mind having your ajax action methods return HTML instead of JSON, they can return the already-constructed UL with its LI children, and you can append that to an element by using data-myapp-response="results". Your success function would then just look like this:
success: function(html) {
var responseId = $(clicked).data('myapp-response');
if (responseId) {
$('#' + responseId).empty().html(html);
}
}
To sum up my best answer for this, if you must return JSON from your action methods, you are skipping the server-side View, so this really isn't server MVC -- it's just MC. If you return PartialViewResult with html to ajax calls, this is server MVC. So if your app must return JSON data for ajax calls, use client MVVM like KnockoutJS.
Either way, I don't like the JS you posted because it mixes your layout (html tags) with behavior (asynchronous data load). Choosing either server MVC with partial html views or client MVVM with pure JSON viewmodel data will solve this problem for you, but manually constructing DOM/HTML in javascript violates separation of concerns.
- Javascript file creation Apparently minification features are coming in .NET 4.5. If you go the unobtrusive route, there shouldn't be anything stopping you from loading all of your JS in 1 script file. I would be careful about creating different JS files for each entity type, you will end up with JS file explosion. Remember, once your script file is loaded, the browser should cache it for future requests.
- Complex queries I don't consider having feature like pagination, sorting, etc, as being complex. My preference is to handle this with URL's and server-side logic, to make the db queries as limited as needed. However we are deployed to Azure, so query optimization is important to us. For example: /widgets/show-{pageSize}-per-page/page-{pageNumber}/sort-by-{sortColumn}-{sortDirection}/{keyword}
. EF and LINQ to Entities can handle pagination and sorting with methods like .Take(), .Skip(), .OrderBy(), and .OrderByDescending(), so you get what you need during the db trip. I haven't found the need for a clientlib yet, so I honestly don't know much about them. Look to other answers for more advice on that.
- Project silk Never heard of this one, will have to check it out. I am a big fan of Steve Sanderson, his books, his BeginCollectionItem HtmlHelper, and his blog. That said, I don't have any experience with KnockoutJS in production. I have checked out its tutorials, but I try not to commit to something until it's at least version 2.0. Like I mentioned, KnockoutJS 2.0 was just released.
- N-tier If by tier you mean different physical machine, then no, I don't think anything goes out any windows. Generally 3-tier means you have 3 machines. So you might have a fat client as your presentation tier, that runs on a user's machine. The fat client might access a service tier, that runs on an application server and returns XML or whatever to the fat client. And the service tier might get its data from a SQL server on a 3rd machine.
MVC is one layer, on 1 tier. Your controllers, models, and views are all part of your Presentation Layer, which is 1 tier in the physical architecture. MVC implements the Model-View-Controller pattern, which is where you might be seeing additional layers. However, try not to think of these 3 aspects as tiers or layers. Try to think of all 3 of them as Presentation Layer Concerns.
Update after pres/bus/data comment
Okay, so you are using tier and layer interchangably. I usually use the term "layer" for logical / project / assembly divisions, and tier for physical network separation. Sorry for the confusion.
You will find quite a few people in the MVC camp who say you should not use the "Models" in MVC for your entity data model, nor should you use your Controllers for business logic. Ideally your models should be view-specific ViewModels. Using something like Automapper, you take your entities from your domain model and DTO them into ViewModels, sculpted specifically for use by the view.
Any business rules should also be part of your domain, and you can implement them using domain services / factory pattern / whatever is appropriate in your domain layer, not in the MVC presentation layer. Controllers should be dumb, though not quite as dumb as models, and should give responsibility to the domain for anything that requires business knowledge. Controllers manage the flow of HTTP requests and responses, but anything with real business value should be above the controller's pay grade.
So, you can still have a layered architecture, with MVC as the presentation layer. It is a client of your application layer, service layer, or domain layer, depending on how you architect it. But ultimately your entity model should be part of the domain, not models in MVC.
for
vs. foreach
There is a common confusion that those two constructs are very similar and that both are interchangeable like this:
foreach (var c in collection)
{
DoSomething(c);
}
and:
for (var i = 0; i < collection.Count; i++)
{
DoSomething(collection[i]);
}
The fact that both keywords start by the same three letters doesn't mean that semantically, they are similar. This confusion is extremely error-prone, especially for beginners. Iterating through a collection and doing something with the elements is done with foreach
; for
doesn't have to and shouldn't be used for this purpose, unless you really know what you're doing.
Let's see what's wrong with it with an example. At the end, you'll find the full code of a demo application used to gather the results.
In the example, we are loading some data from the database, more precisely the cities from Adventure Works, ordered by name, before encountering "Boston". The following SQL query is used:
select distinct [City] from [Person].[Address] order by [City]
The data is loaded by ListCities()
method which returns an IEnumerable<string>
. Here is what foreach
looks like:
foreach (var city in Program.ListCities())
{
Console.Write(city + " ");
if (city == "Boston")
{
break;
}
}
Let's rewrite it with a for
, assuming that both are interchangeable:
var cities = Program.ListCities();
for (var i = 0; i < cities.Count(); i++)
{
var city = cities.ElementAt(i);
Console.Write(city + " ");
if (city == "Boston")
{
break;
}
}
Both return the same cities, but there is a huge difference.
- When using
foreach
, ListCities()
is called one time and yields 47 items.
- When using
for
, ListCities()
is called 94 times and yields 28153 items overall.
What happened?
IEnumerable
is lazy. It means that it will do the work only at the moment when the result is needed. Lazy evaluation is a very useful concept, but has some caveats, including the fact that it's easy to miss the moment(s) where the result will be needed, especially in the cases where the result is used multiple times.
In a case of a foreach
, the result is requested only once. In a case of a for
as implemented in the incorrectly written code above, the result is requested 94 times, i.e. 47 × 2:
Querying a database 94 times instead of one is terrible, but not the worse thing which may happen. Imagine, for example, what would happen if the select
query would be preceded by a query which also inserts a row in the table. Right, we would have for
which will call the database 2,147,483,647 times, unless it hopefully crashes before.
Of course, my code is biased. I deliberately used the laziness of IEnumerable
and wrote it in a way to repeatedly call ListCities()
. One can note that a beginner will never do that, because:
The IEnumerable<T>
doesn't have the property Count
, but only the method Count()
. Calling a method is scary, and one can expect its result to not be cached, and not suitable in a for (; ...; )
block.
The indexing is unavailable for IEnumerable<T>
and it's not obvious to find the ElementAt
LINQ extension method.
Probably most beginners would just convert the result of ListCities()
to something they are familiar with, like a List<T>
.
var cities = Program.ListCities();
var flushedCities = cities.ToList();
for (var i = 0; i < flushedCities.Count; i++)
{
var city = flushedCities[i];
Console.Write(city + " ");
if (city == "Boston")
{
break;
}
}
Still, this code is very different from the foreach
alternative. Again, it gives the same results, and this time the ListCities()
method is called only once, but yields 575 items, while with foreach
, it yielded only 47 items.
The difference comes from the fact that ToList()
causes all data to be loaded from the database. While foreach
requested only the cities before "Boston", the new for
requires all cities to be retrieved and stored in memory. With 575 short strings, it probably doesn't make much difference, but what if we were retrieving only few rows from a table containing billions of records?
So what is foreach
, really?
foreach
is closer to a while loop. The code I previously used:
foreach (var city in Program.ListCities())
{
Console.Write(city + " ");
if (city == "Boston")
{
break;
}
}
can be simply replaced by:
using (var enumerator = Program.ListCities().GetEnumerator())
{
while (enumerator.MoveNext())
{
var city = enumerator.Current;
Console.Write(city + " ");
if (city == "Boston")
{
break;
}
}
}
Both produce the same IL. Both have the same result. Both have the same side effects. Of course, this while
can be rewritten in a similar infinite for
, but it would be even longer and error-prone. You're free to choose the one you find more readable.
Want to test it yourself? Here's the full code:
using System;
using System.Collections.Generic;
using System.Data;
using System.Data.SqlClient;
using System.Diagnostics;
using System.Linq;
public class Program
{
private static int countCalls;
private static int countYieldReturns;
public static void Main()
{
Program.DisplayStatistics("for", Program.UseFor);
Program.DisplayStatistics("for with list", Program.UseForWithList);
Program.DisplayStatistics("while", Program.UseWhile);
Program.DisplayStatistics("foreach", Program.UseForEach);
Console.WriteLine("Press any key to continue...");
Console.ReadKey(true);
}
private static void DisplayStatistics(string name, Action action)
{
Console.WriteLine("--- " + name + " ---");
Program.countCalls = 0;
Program.countYieldReturns = 0;
var measureTime = Stopwatch.StartNew();
action();
measureTime.Stop();
Console.WriteLine();
Console.WriteLine();
Console.WriteLine("The data was called {0} time(s) and yielded {1} item(s) in {2} ms.", Program.countCalls, Program.countYieldReturns, measureTime.ElapsedMilliseconds);
Console.WriteLine();
}
private static void UseFor()
{
var cities = Program.ListCities();
for (var i = 0; i < cities.Count(); i++)
{
var city = cities.ElementAt(i);
Console.Write(city + " ");
if (city == "Boston")
{
break;
}
}
}
private static void UseForWithList()
{
var cities = Program.ListCities();
var flushedCities = cities.ToList();
for (var i = 0; i < flushedCities.Count; i++)
{
var city = flushedCities[i];
Console.Write(city + " ");
if (city == "Boston")
{
break;
}
}
}
private static void UseForEach()
{
foreach (var city in Program.ListCities())
{
Console.Write(city + " ");
if (city == "Boston")
{
break;
}
}
}
private static void UseWhile()
{
using (var enumerator = Program.ListCities().GetEnumerator())
{
while (enumerator.MoveNext())
{
var city = enumerator.Current;
Console.Write(city + " ");
if (city == "Boston")
{
break;
}
}
}
}
private static IEnumerable<string> ListCities()
{
Program.countCalls++;
using (var connection = new SqlConnection("Data Source=mframe;Initial Catalog=AdventureWorks;Integrated Security=True"))
{
connection.Open();
using (var command = new SqlCommand("select distinct [City] from [Person].[Address] order by [City]", connection))
{
using (var reader = command.ExecuteReader(CommandBehavior.SingleResult))
{
while (reader.Read())
{
Program.countYieldReturns++;
yield return reader["City"].ToString();
}
}
}
}
}
}
And the results:
--- for ---
Abingdon Albany Alexandria Alhambra [...] Bonn Bordeaux Boston
The data was called 94 time(s) and yielded 28153 item(s).
--- for with list ---
Abingdon Albany Alexandria Alhambra [...] Bonn Bordeaux Boston
The data was called 1 time(s) and yielded 575 item(s).
--- while ---
Abingdon Albany Alexandria Alhambra [...] Bonn Bordeaux Boston
The data was called 1 time(s) and yielded 47 item(s).
--- foreach ---
Abingdon Albany Alexandria Alhambra [...] Bonn Bordeaux Boston
The data was called 1 time(s) and yielded 47 item(s).
LINQ vs. traditional way
As for LINQ, you may want to learn functional programming (FP) - not C# FP stuff, but real FP language like Haskell. Functional languages have a specific way to express and present the code. In some situations, it is superior to non-functional paradigms.
FP is known being much superior when it comes to manipulating lists (list as a generic term, unrelated to List<T>
). Given this fact, the ability to express C# code in a more functional way when it comes to lists is rather a good thing.
If you're not convinced, compare the readability of code written in both functional and non-functional ways in my previous answer on the subject.
Best Answer
LINQ is primarily designed to allow pure functional queries and transformations on sequences of data (you will notice that all the LINQ extensions take Func delegates but not Action delegates). Consequently the most common case of a loop that does not fit with LINQ very well is one that is all about non-pure functional side effects, e.g.
To get better at using LINQ, just practice using it.
Every time you are about to write a
for
orforeach
loop to do something with a collection, stop, consider if it's a good fit for LINQ (i.e. it's not just performing an action/side effect on the elements), and if so force yourself to write it using LINQ.You could also write the
foreach
version first then rewrite to a LINQ version.As svick points out, LINQ should be about making your program more readable. It is usually good at this as it tends to emphasize the intent of the code rather than the mechanism; however if you find you cannot make your queries more readable than a simple loop, feel free to stick with the loop.
If you need exercises to practice, most functional programming exercises will map nicely to LINQ e.g. 99 problems (especially the first 20 or so) or project euler.