normalize-crlf.ps1

I know it is not part of your main code, but still have to give you a review :P You touch every file, no matter if they need changing or not...

Use this in the inner loop to check (It is a bit overkill to read the whole file, but you are converting small files)

$fn = $_.FullName<br/> if ([System.IO.File]::ReadAllText($fn).Contains(";`n"))<br/> {<br/> ( Get-Content $fn -ReadCount 9999999 ) | Set-Content $fn<br/> }

And now your blog has became self-perpetuating: You blog about creating a blog.

just a small thing:

    public static string ResolveTags(string[] tags)
    {
        string result = tags.Aggregate(string.Empty, (current, tag) => current + (TagsSeparator + tag));
        return result.Trim(TagsSeparator);
    }

Can be written as:

    public static string ResolveTags(string[] tags)
    {
        return string.Join(TagsSeparator.ToString(), tags).Trim(TagsSeparator);
    }

ToString is only needed, if the TagsSeparator is char and not string. Simple tests show, its 4 times faster. I know i doesnt really matter much, but i think join is easier to understand than an aggregate.

I just came accros this piece of code in PostDetailsController (line 48/49):

        if (vm.Post.Slug != slug)
            return RedirectToActionPermanent("Details", new { id, vm.Post.Slug });

This can be moved to line 33. and written as:

        var postslug = SlugConverter.TitleToSlag(post.Title)
        if (postslug != slug)
            return RedirectToActionPermanent("Details", new { id, postslug });

this avoids the unneccesary loading of comments and creation/mapping of the viewmodel (which is not used because of the redirect).

also note the spelling mistake in TitleToSlag :-)

There is also a double redirect, meaning unnecessary requests from the server. For example a request to:

http://ayende.com/Blog/archive/2009/03/08/designing-a-document-database.aspx

which is a search result in Google, will first respond with 301 to:

http://ayende.com/blog/archive/2009/03/08/designing-a-document-database.aspx

and a request to that page will respond with a 301 to the now real URL:

/blog/3897/designing-a-document-database

And now, writing this post, the post preview doesn't seem to handle URLs. Hopefully the posted comment will not look to awful...

I'm thinking someone should review the future post code; as they don't seem to be automatically posting properly ;)

"double redirect" this is what I meant when I was talking about SEO. redirect chains are counterintuitively bad for propagating the accumulated trust the old page had.

is it just me, or there's no unit tests?

@Giedrius Banaitis

http://ayende.com/blog/4092/reviewing-nerddinner

https://github.com/ayende/RaccoonBlog/blob/master/src/RaccoonBlog.IntegrationTests/Views/Post/Details.cs

;-) let's face it, who wants to code a few tens of lines of code (I am counting code to define the test fixture, usings and all the boiler plate code) to test that a method with "return new Something()" really returns Something.

There should be somebody who has balls to admin this :-))

PostAdminController.CommentsAdmin is marked by NDepend as being fairly ugly. I have to say, I agree.

Since you do seem to like using readonly as well, in the AskimetService you can set the key to readonly, and get rid of the document session instance var.

There are a couple other findings, but all in all pretty undramatic.

@Steves - well there are several things: a) first first class i've opened breaks the rantings posted on your link - I mean several classes in same file and one of them is controller? b) both commands have more than 10 lines (although there's no such rule - if there's more than 10 lines - test it), so i think they are quite complicated, first of all, because they would be not easy testable without refactoring (because you would struggle mocking smtp client or akismet service). All this ranting from my side is just because i see different approach in Ayendes code (talking about this blog engine only, haven't seen much code on other his projects) - he is not trying to make everything mockable and easy testable, question is if that is a good idea.

@Ayende

I understand you want to keep things simple.

But look at PostAdminController:

In the Details(...) method this construct is also used (first map post to viewmodel. then check generated slug against slug parameter)

This is good in the sence that it hides the slug generation logic (it's deep hidden in a automapper profile, I don't like that either, but that's not my point right now).

Moving along to line 132 of the same controller (in the 'ugly' (not my words) CommentsAdmin method). SlugConverter is used directly!

So I don't really get why you don't want to use this SlugConverter directly in the other methods to gain some performance.

Remco

@Ayende

I am checking the structure of the solution, and trying to understand how you organize your artifacts.

I am confused about the Common, Infrastructure and Helpers folder.

For example, - why you put the class LowercaseRoute in the Common folder instead of the Infrastrucure folder. - why you put the action filter AjaxOnlyAttribute in the Helpers\Attributes folder and the RavenActionFilter inside Infrastructure\Controllers

One more question: the Services folder contains both what you are exposing (MetaWeblog) and consuming (Askimet) right? I think the term Service is very ambiguous. For example, let's say that I have a LoggingService: would you put the class inside the Infrastructure folder or Services folder?

My concern about the solution structure is to have all team members aligned, and not have think too much where should I put some class? My team is checking RaccoonBlog trying to learn that it is possible to have things well done without an heavy architecure (repositories, dependency injection, too mucch tiers, etc.)