DevMavens Feed

A bad test

Oren Eini — Fri, 18 May 2012 13:00:00 GMT

This is a bad test, because what it does is ensuring that something does not works. I just finished implementing the session.Advaned.Defer support, and this test got my attention by failing the build.

Bad test, you should be telling me when I broke something, not when I added new functionality.

Common Design Patterns Presentations

Steve Smith — Thu, 17 May 2012 17:18:16 GMT

This month I presented on Common Design Patterns at two regional events. I’ve given this talk a few times before, and it continues to evolve. The first one was on 4 May at Stir Trek, as a replacement talk for a cancellation. I only had about 24 hours’ notice so I didn’t spend much time updating the talk, and in fact I had to rip out a bunch of stuff that was specific to the last event I gave it at or outdated information, so visually the slides at Stir Trek weren’t what I would consider up to par. The second time I presented was last night at MIGANG in Michigan, and I had a chance to give the slides a bit more love and choose some appropriate imagery to support the concepts. You can view the slides from the presentation (the second one) here:

Common design patterns (migang 16 May 2012)

View more presentations from Steven Smith.

The presentation’s demo code is actually a fork of the MVC Music Store sample available on CodePlex. You can download it as a zip or via Mercurial from the MvcMusicStoreRepositoryPattern fork here.

I did realize after talking to a couple of attendees last night that at one point I mentioned that the Design Patterns book was by Fowler. I meant Gang of Four. If you’re looking for good books on the patterns I discussed, and others, here are some good ones:

Design Patterns – reference by Gang of Four
Head First Design Patterns – great learning book, highly recommended
Patterns of Enterprise Application Architecture – more patterns for use in enterprise applications by Fowler
Domain Driven Design – builds on and introduces additional patterns
Refactoring – reference by Fowler
Working Effectively with Legacy Code – in my opinion this shows better how, when, and why to refactor code than Refactoring does

And of course if you want to learn Design Patterns and the Principles of Object Oriented Design that they follow, you’ll find great Pluralsight courses on these topics.

When using the Task Parallel Library, Wait() is a BAD warning sign

Oren Eini — Thu, 17 May 2012 13:00:00 GMT

Take a look at the following code:

public static Task ParseAsync(IPartialDataAccess source, IPartialDataAccess seed, Stream output, IEnumerable<RdcNeed> needList)
{
    return Task.Factory.StartNew(() =>
    {
        foreach (var item in needList)
        {
            switch (item.BlockType)
            {
                case RdcNeedType.Source:
                    source.CopyToAsync(output, Convert.ToInt64(item.FileOffset), Convert.ToInt64(item.BlockLength)).Wait();
                    break;
                case RdcNeedType.Seed:
                    seed.CopyToAsync(output, Convert.ToInt64(item.FileOffset), Convert.ToInt64(item.BlockLength)).Wait();
                    break;
                default:
                    throw new NotSupportedException();
            }
        }
    });
}

Do you see the problem in here?

It is a result of a code review comment about improper use of async in a project. This resulted in a lot of Task showing up in the return methods, but not in any measurable improvement in the actual codebase use of asynchronicity.

The problem is that when you need to work with such things in C# 4.0, you have to do some annoying things to get the code to work properly. In particular, this method was modified to be:

public static Task ParseAsync(IPartialDataAccess source, IPartialDataAccess seed, Stream output, IList<RdcNeed> needList, int position = 0)
{
  if(position>= needList.Count)
  {
        return new CompletedTask();
  }
  var item = needList[position];
  Task task;
            
  switch (item.BlockType)
  {
        case RdcNeedType.Source:
            task = source.CopyToAsync(output, Convert.ToInt64(item.FileOffset), Convert.ToInt64(item.BlockLength));
            break;
        case RdcNeedType.Seed:
            task = seed.CopyToAsync(output, Convert.ToInt64(item.FileOffset), Convert.ToInt64(item.BlockLength));
            break;
        default:
            throw new NotSupportedException();
  }

  return task.ContinueWith(resultTask =>
    {
        if (resultTask.Status == TaskStatus.Faulted)
            resultTask.Wait(); // throws
        return ParseAsync(source, seed, output, needList, position + 1);
    }).Unwrap();
}

This code is more complex, but it is actually making proper use of the TPL. We have changed the loop into a recursive function, so we can take advantage of ContinueWith to the next iteration of the loop.

And no, I can’t wait to get to C# 5.0 and have proper await work.

Please Learn to Think about Abstractions

Scott Hanselman — Thu, 17 May 2012 04:05:18 GMT

Jeff Atwood wrote a post called Please Don't Learn to Code and Zed Shaw wrote a post called Please Don't Become Anything, Especially Not A Programmer. My wife lost her wedding ring down the drain. She freaked out and came running declaring that it

Why I Taught My Daughter To Code (A Little)

Jon Galloway — Thu, 17 May 2012 02:11:37 GMT

Literacy

Imagine a world in which very few people knew how to read or write. You kept to certain parts of town because you couldn't read a map or a street sign. When you needed to sign a contract, you just asked what it said and had to take it on faith. A lot of your experience was based on legend and rumor. Books, and the information in them, were mystical. Sometimes you suspected they were being used against you, but you never really knew.

There was definitely plenty of work around, but some high end jobs weren't remotely possible - not just clerical work, but professions that required a lot of information management, like the medical and legal professions.

Please don't tell me not to learn to code

Over this past year, there have been commentary back and forth on whether everyone should learn to code. A few free, interactive sites like Codecademy popped up which made it easy to start learning some basic coding. I was a big fan - I helped my eleven year old daughter go through it, and we both agreed it was great. More on that later.

Then Codecademy launched Code Year - a grand plan to teach hundreds of thousands of people to code. It was a big hit - even NYC's mayor signed up.

But no good deed goes unpunished, and there's been some snarky criticism from - you guessed it - professional coders. The latest if Jeff Atwood's post, Please Don't Learn To Code.

Hogwash. Learning some basic coding is an excellent investment of anyone's time.

Easy on the Straw Men

It's "please don't hear what I'm not saying" time.

I don't think that the general populace needs to be proficient at writing code. It would be silly to argue Mayor Bloomberg shouldn't be slinging Javascript on the job (as Jeff does). That's not at all the goal. Nobody's saying that everyone should become a programmer.

What I - and others -are saying is that learning some basic coding skills is an excellent investment of anyone's time. I'll make two arguments:

Understanding computers (including all the computing devices and products that surround us) is of huge and growing benefit to everyone.
Basic programming is an excellent way to gain some of that understanding.

Computer Literacy as a Basic Life Skill - Right Now

Right now, pretty much every part of life is in some way affected by computers - most pretty heavily. Shopping, socializing, medicine, education, law, entertainment and more are all at the very least affected by computers and the internet. My three daughters had a use for basic computer skills long before math would do them any good.

This only gets more intense with increasing age. Teenagers can greatly profit from computer expertise and tech savvy. It's hard to imagine a college program (at least one with any practical application) that didn't involve a hefty amount of computer usage - from basic internet research and word processing on up to simulation and research.

Most careers (in the industrial world) involve some amount of information management, and workers in these professions who have some computer skill can often be a lot more effective. That includes areas where even a decade ago people might not have expected to see computers - agriculture, construction, family-run restaurants.

I've talked with countless "civilians" who have profited from computer skills over the years - a real estate appraiser who figured out how to automate using Excel and worked circles around his colleagues, film makers and musicians who figured out how to record, market and distribute on the internet, non-techies who sell things online, a school facilities manager who figured out how to cut costs significantly using computer security and automation.

And many of those people had new doors and careers open to them due to their new-found computer skills. When the real estate market slowed down, the appraiser jumped right into a technology job. Heck, learning to code on my own - starting with my grade school's Apple II - gave me the option to jump into a computer career after I got out of the Navy (in a technical but not computer related field).

Computer Literacy as an Absolute Essential in the Very Near Future

"The future is already here — it's just not very evenly distributed."

- William Gibson

It should be obvious to anyone paying attention that computers have become a huge part of our daily lives, and that trend is rapidly accelerating.

I'm reading Physics of the Future (Michio Kaku). It's a fascinating book in which Michio points to obvious trends and extrapolates where things are going in the near future. In many cases, even a very conservative estimate - or even a look around - shows that every part of life will soon involve interacting with computers and the internet. He points out that global powers during World War II would have killed for the computational power in a simple musical birthday card. Now, we just throw them away. Average mobile phones rival supercomputers from the not-too-distant past, and they're connected to a growing internet and GPS satellites. In the next few decades, computers will continue to shrink and become integrated into everything we interact with.

The quality of our everyday lives will be greatly affected by how we interact with computers. How should that interaction take place?

Magical! Or, Should We See Computers as Products or Tools?

Technology companies work hard to package technology so that it can be used naturally, so consumers won't have to think or fiddle with details. This is a good thing. Anyone working in technology should be focused, consumed with the challenge of exposing the possible as the natural. We should be doing this not just because it's a worthy goal, but because it makes good business sense - friendly products sell.

But it is not in the average consumer's interest for them to blindly buy into this magical experience.

Think about the historical progress in how informed consumers approach medicine. In ancient times, mystics sold magic and consumers had no real insight into what they were buying. Over the centuries, the public has gained a much more informed view of science and medicine, and this is of huge benefit to them as consumers. We expect to see evidence, we read about product recalls, and the market for doctors who sell amulets and snake oils is pretty small. This applies to other areas of life, as well. While I could get around in a car I thought was powered by tiny horses, I'd be unlikely to make wise decisions regarding the purchase and repairs to said car. An informed approach to modern life helps.

Now think about the consumer approach to computer products.

Think about how computers and technology are shown in movies and TV.

Think about how helpless your relatives and neighbors feel when they're buying techy products, or when something doesn't work.

Think about a friend or relative who paid too much for a website that never worked right.

Ah, but you may be thinking about a favorite company who sells you magical products right now, or runs some of your favorite websites, or sells you apps or movies. Well marketed brands that have built reputations for quality and ease of use have helped uninformed consumers navigate the technology landscape, but keep in mind that these companies have divided interests. It's their job. They want to sell you things, and their stockholders demand that they bring in as much profit as they can from those sales. Mindless and uninformed brand loyalty is not only expensive, it's by nature counter to your interests. It's no way to live - and thrive - in a digital world.

Computer Literacy as a Leadership Requirement

The basic level of technical understanding that we expect of our leaders must grow with time in order for them to govern. We expect them to understand our legal system, but we also expect they should know that medicines are chemicals which influence the biological processes in our bodies, that electricity powers lightbulbs and is transmitted over an electrical power grid, that mobile phones have radios in them, etc. I'd be scared to hear that our leadership didn't have a very basic understanding of our modern world. They write and enforce laws and policies that greatly impact our lives, so they need to have a clue about the world we live in.

In the same way, leaders who see computers as magic boxes, coders as wizards, and computer code as incantations can't effectively govern. They can't set policy, they can't have informed legal opinions, they have no framework for evaluating whether information being fed to them is makes any sense (or even knowing if they should ask).

Leadership requires understanding, and more, it requires - well, a sense of direction. The future is going to include more and more technology, and leaders who don't understand the present or even have a rough idea of the road ahead are a menace.

Computer Literacy as an Educational Oversight

I've been describing why non-techies can benefit from some computer literacy. Unfortunately, they won't get it in from our schools.

Computer literacy is undoubtedly of more use to most people than the foreign languages most high schools require (I want those 4 years I spent on high school French back). I have rich conversations with people worldwide thanks to the internet and automated translations, not any time I've spent studying languages.

Though it may offend, computer skills are of more use to the huge majority than mathematics including and beyond first year algebra. Most people will rarely if ever use algebra; everyone uses computers.

Consider this: in the off chance that the average farmer / dentist / lawyer / car mechanic / stay at home parent / author / teacher / etc. needs to solve an algebra problem, they can punch it into the search engine and get the answer: 3x + 7y = 4y - 35. Or, more likely, just ask their phone.

The same goes for so many things we devote educational time to. Does the average high school graduate remember anything important about titration or Augustus or sentence diagram? Titian or Taft? Or even remember how WWI started? Does it matter when they can just look it up on Wikipedia?

I'm all in favor a balanced education. I got a B.S. Physics (with honors) from Annapolis and went through the US Navy Nuclear Power program (a very rigorous engineering program) following that. My parents were both school teachers. My wife and I homeschool our daughters and put a very high priority on their education. I see education is important, and I think STEM is an important priority.

But in the broader perspective, our educational priorities are often focused on a historical ideal, completely out of touch with a world in which nearly everyone interacts regularly with computers that most people barely understand.

This is a much bigger discussion, but here's the main takeaway: after thousands of hours in the classroom, our students emerge unempowered into an increasingly digital world. We're on our own here.

Excess Capacity: Oh, the Hours We Throw Away

Fine, most people could benefit from some computer literacy. But who has the time?

I don't buy it. We as a society throw away countless hours on games, entertainment, and apparently even tracking the adventures of someone called Snooki. We make video mashups and silly tumblogs and meme pictures of cats. We catalog Pokémon. We read books about vampires romancing high school girls. We write long blog posts.

We have the time.

We have a ridiculous amount of leisure time, hours we just throw away because we can't think of anything better to do.

Basic Coding as a Gateway To Basic Computer Literacy

While it might make sense, I'm not proposing citizens spend a mandatory two years in the "Get Smart About Computers" corps. I'm just proposing that:

Non-techies consider spending a few hours learning how computers work.
Techies encouraging them - and not actively discouraging them ! Sheesh!

Thankfully, getting a basic idea of how computers work has gotten incredibly easy. Sites like Codecademy make it easy to learn - hands on - how computers work. These lessons are hands-on, and they're practical. You learn web fundamentals and how to build a simple website. This is empowering stuff. It's a great way to start the journey from a powerless consumer to a contributor, or at least informed consumer. It's not time consuming, and it's free.

Maybe some of the negative reactions from coders to the Code Year phenomenon have come from not bothering to actually read what's in the classes and assuming that the goal was to turn a bunch of people into programmers. Maybe.

Case Study: Rosemary, Champion of the World (and my daughter)

This past year, my daughter asked me if I could teach her computers. I talked to my wife and rolled it into her school plan. We had ten lessons. During this time, we worked did the following:

We worked through the Getting Started With Programming section of the Codecademy Javascript Fundamentals course. She did all the work on her own, and when she got stuck I would have her re-read the instructions out loud. She'd say "Oh! I should have read more carefully!" and move on. She learned some basic loops and conditionals - nothing difficult. I gave her at test on repl.it where she wrote a loop from 1 to 25, outputting either "You are too young to drive a car!" or "Your age is 20, you can drive a car!"
She asked how the internet works, so we watched a video on YouTube about how the internet works. Then we went on a field trip to our router, and we talked about how to "fix the internet" when it's not working.
She asked about making a web page, and how web pages work. We played with browser tools (Chrome and IE) and she learned the difference between HTML, CSS, and Javascript. I let her recolor my webpage pink, and we went to CNN and put her picture and the headline of her choice in the headlines.
She asked about making a computer game. We talked about why that would take some work, and planned to do that when we started back up in the semester.

This was fun. We both had a good time. She asked questions that wouldn't have come up otherwise, and she wouldn't have been interested in otherwise.

More importantly, after only 8 or so hours, I noticed she saw the world differently. She had ideas about what computers should do. She laughed when computers did silly things on TV. She read error messages and solved problems in an intelligent way. She keeps asking about building a computer game.

She understands that there's no reason that girls can't have fun with computers, too.

From a time investment of about 8 hours.

This started with writing some Javascript and HTML. That was critical because it helped her understand how computers work. She learned that they were unthinking and unforgiving in the way they followed her instructions. More importantly, she learned that they would do what she told her if she learned their language (in this case, some basic Javascript).

Case Study: The Judge

In a funny bit of timing, just last night I read about the judge on the Oracle / Google trial was able to make an informed decision about a Java function because he'd been -shocker - learning some Java. It was something that would have seemed obvious to most programmers, but was delightfully refreshing to hear in a court.

Now here is a later followup where the Judge slams Oracle:

Judge: We heard the testimony of Mr. Bloch. I couldn't have told you the first thing about Java before this problem. I have done, and still do, a significant amount of programming in other languages. I've written blocks of code like rangeCheck a hundred times before. I could do it, you could do it. The idea that someone would copy that when they could do it themselves just as fast, it was an accident. There's no way you could say that was speeding them along to the marketplace. You're one of the best lawyers in America, how could you even make that kind of argument?

Oracle: I want to come back to rangeCheck.

Judge: rangeCheck! All it does is make sure the numbers you're inputting are within a range, and gives them some sort of exceptional treatment. That witness, when he said a high school student could do it...

So here's a judge who's able to make an informed decision about a technology case. What's more shocking is that this is news.

Some Objections

Shouldn't everyone learn plumbing, too?

Jeff's been having fun equating the "learn to code" movement with "learn plumbing!"

This is one of those fun arguments that sounds good but falls apart after a moment of thought. What would the return on investment be - both in cost and time - for your average person to learn more about plumbing than working a plunger? How often do they work with plumbing in a way that would benefit from a deeper understanding?

How often do most people work with computers, smartphones, and internet sites in a way that would benefit from knowing a bit more?

Not more code! The world needs less code!

This is a bad argument for three reasons:

This is a coder's argument to other coders. Solving your application's issues is often best solved by thinking, not mindless coding - granted. That has nothing to do with whether it's worthwhile for your average person to learn about how computers work.
It's - perhaps unintentionally - elitist. We nerds have a huge advantage in dealing with the computers that surround us. Telling the rest of the world that they can't join in, or acting like it's harder than it is, is just wrong.
It assumes that amateurs won't be writing code anyways. I've seen tons of code over the years written by people who had a problem to solve. They cobbled it together from internet searches. It was inelegant, but it worked in that it solved their problem. There are a lot of populist pseudocoding tools out there - think of ifttt or the use of countless Wordpress plugins - that let real people solve real problems without having to wait for nerds to get involved. They'll do what they need to do, the question is whether we'll encourage them and help them find the structure.

Fine, learn computer literacy, not coding!

This is at least something. The problem here is twofold:

It assumes people know what questions to ask, or how to ask them. As long as they see computers and the internet as mystical boxes full of magic, that's not the case. I'm continually helping people solve problems by prompting them to actually read what a program or web page is telling them (and saying that computers and web pages need to give better messages or not require input is irrelevant).

Without knowing the fundamental differences between how computers and people think, people resort to just yelling louder in their own language.
It assumes we know what people need to learn. That's only kind of true. Often the approaches I've seen here come down to teaching them how to use a particular product or software package, which is definitely not "teaching a man to fish." Real problem solving skills in the technology world come down to thinking systematically and rationally about technology, which requires some basic understanding.

Where Next?

If you don't know how to code, get started. It's free, fun and easy.

If you know how to code, encourage others around you when they want to learn. Odds are, someone did that for you a while back.

DropDownList and SelectListItem Array Item Updates in MVC

Rick Strahl — Wed, 16 May 2012 20:44:00 GMT

So I ran into an interesting behavior today as I deployed my first MVC 4 app tonight. I have a list form that has a filter drop down that allows selection of categories. This list is static and rarely changes so rather than loading these items from the database each time I load the items once and then cache the actual SelectListItem[] array in a static property.

However, when we put the site online tonight we immediately noticed that the drop down list was coming up with pre-set values that randomly changed. Didn't take me long to trace this back to the cached list of SelectListItem[]. Clearly the list was getting updated - apparently through the model binding process in the selection postback.

To clarify the scenario here's the drop down list definition in the Razor View:

@Html.DropDownListFor(mod => mod.QueryParameters.Category, Model.CategoryList, "All Categories")

where Model.CategoryList gets set with:

[HttpPost]
[CompressContent]
public ActionResult List(MessageListViewModel model)
{
    InitializeViewModel(model);

    busEntry entryBus = new busEntry();
    var entries = entryBus.GetEntryList(model.QueryParameters);

    model.Entries = entries;
    model.DisplayMode = ApplicationDisplayModes.Standard;
       
    model.CategoryList = AppUtils.GetCachedCategoryList();

    return View(model);
}

The AppUtils.GetCachedCategoryList() method gets the cached list or loads the list on the first access. The code to load up the list is housed in a Web utility class. The method looks like this:

/// <summary>
/// Returns a static category list that is cached
/// </summary>
/// <returns></returns>
public static SelectListItem[] GetCachedCategoryList()
{
    if (_CategoryList != null)
        return _CategoryList;

    lock (_SyncLock)
    {
        if (_CategoryList != null)
            return _CategoryList;
        
        var catBus = new busCategory();
        var categories = catBus.GetCategories().ToList();


        // Turn list into a SelectItem list            
        var catList= categories
                         .Select(cat => new SelectListItem() { Text = cat.Name, Value = cat.Id.ToString() })
                         .ToList();

        catList.Insert(0, new SelectListItem()
        {
            Value = ((int)SpecialCategories.AllCategoriesButRealEstate).ToString(),
            Text = "All Categories except Real Estate"
        });
        catList.Insert(1, new SelectListItem()
        {
            Value = "-1",
            Text = "--------------------------------"                    
        });
        
        _CategoryList = catList.ToArray();
    }

    return _CategoryList;
}
private static SelectListItem[] _CategoryList ;

This seemed normal enough to me - I've been doing stuff like this forever caching smallish lists in memory to avoid an extra trip to the database. This list is used in various places throughout the application - for the list display and also when adding new items and setting up for notifications etc..

Watch that ModelBinder!

However, it turns out that this code is clearly causing a problem. It appears that the model binder on the [HttpPost] method is actually updating the list that's bound to and changing the actual entry item in the list and setting its selected value. If you look at the code above I'm not setting the SelectListItem.Selected value anywhere - the only place this value can get set is through ModelBinding. Sure enough when stepping through the code I see that when an item is selected the actual model - model.CategoryList[x].Selected - reflects that.

This is bad on several levels: First it's obviously affecting the application behavior - nobody wants to see their drop down list values jump all over the place randomly. But it's also a problem because the array is getting updated by multiple ASP.NET threads which likely would lead to odd crashes from time to time. Not good!

In retrospect the modelbinding behavior makes perfect sense. The actual items and the Selected property is the ModelBinder's way of keeping track of one or more selected values. So while I assumed the list to be read-only, the ModelBinder is actually updating it on a post back producing the rather surprising results. Totally missed this during testing and is another one of those little - "Did you know?" moments.

So, is there a way around this? Yes but it's maybe not quite obvious. I can't change the behavior of the ModelBinder, but I can certainly change the way that the list is generated. Rather than returning the cached list, I can return a brand new cloned list from the cached items like this:

/// <summary>
/// Returns a static category list that is cached
/// </summary>
/// <returns></returns>
public static SelectListItem[] GetCachedCategoryList()
{
    if (_CategoryList != null)
    {
        // Have to create new instances via projection
        // to avoid ModelBinding updates to affect this
        // globally
        return _CategoryList
                    .Select(cat => new SelectListItem()
                                   {
                                       Value = cat.Value,
                                       Text = cat.Text
                                   })
                    .ToArray();
    }

…

The key is that newly created instances of SelectListItems are returned not just filtered instances of the original list. The key here is 'new instances' so that the ModelBinding updates do not update the actual static instance. The code above uses LINQ and a projection into new SelectListItem instances to create this array of fresh instances. And this code works correctly - no more cross-talk between users.

Unfortunately this code is also less efficient - it has to reselect the items and uses extra memory for the new array. Knowing what I know now I probably would have not cached the list and just take the hit to read from the database. If there is even a possibility of thread clashes I'm very wary of creating code like this. But since the method already exists and handles this load in one place this fix was easy enough to put in.

Live and learn. It's little things like this that can cause some interesting head scratchers sometimes…

Posted in MVC ASP.NET .NET

File System Wrappers for Unit Testing in C#

Roy Osherove — Wed, 16 May 2012 16:29:18 GMT

I’ve asked this on twitter, and here’s what I got:

System.IO.Abstractions (looks pretty deep)
SystemWrapper (I’m not crazy about the file system being used as a mock API)
Something older

I’d take a look at the first on the list first, if only for it being on github as a sign of advancement.

Test Driven Validation Logic with Extract & Override

Roy Osherove — Wed, 16 May 2012 13:24:38 GMT

In the NerdDinner project, there’s this amazing test (see the bottom, second one).

There are so many things wrong here:

What is exactly correct about these properties?
Many magic values
Multiple tests in a single test
if it fails, its hard to find out why

Here’s how I would approach this kind of validation logic:

use Extract and override, with the test setting the flags for all ‘other’ validation functions except the one I’m testing right now. and I’d at least have two tests for each validation logic. This can also be accomplished with multiple classes and an interface, but I find the tests this way are more readable.

Reviewing RavenDB app: ReleaseCandidateTracker

Oren Eini — Wed, 16 May 2012 13:00:00 GMT

ReleaseCandidateTracker is a new RavenDB based application by Szymon Pobiega. I reviewed version 5f7e42e0fb1dea70e53bace63f3e18d95d2a62dd. At this point, I don’t know anything about this application, including what exactly it means, Release Tracking.

I downloaded the code and started VS, there is one project in the solution, which is already a Good Thing. I decided to randomize my review approach and go and check the Models directory first.

Here is how it looks:

This is interesting for several reasons. First, it looks like it is meant to keep a record of all deployments to multiple environments, and that you can lookup the history of each deployment both on the environment side and on the release candidate side.

Note that we use rich models, which have collections in them. In fact, take a look at this method:

Which calls to this method:

You know what the really fun part about this?

It ain’t relational model. There is no cost of actually making all of these calls!

Next, we move to the Infrastructure folder, where we have a couple of action results and the RavenDB management stuff. Here it how RCT uses RavenDB:

public static class Database
{
    private static IDocumentStore storeInstance;

    public static IDocumentStore Instance
    {
        get
        {
            if (storeInstance == null)
            {
                throw new InvalidOperationException("Document store has not been initialized.");
            }
            return storeInstance;
        }
    }

    public static void Initialize()
    {
        var embeddableDocumentStore = new EmbeddableDocumentStore {DataDirectory = @"~\App_Data\Database"};
        embeddableDocumentStore.Initialize();
        storeInstance = embeddableDocumentStore;
    }
}

It is using an embedded database to do that, which makes it very easy to use the app. Just hit F5 and go. In fact, if we do, we see the fully functional website, which is quite awesome .

Let us move to seeing how we are managing the sessions:

public class BaseController : Controller
{
    public IDocumentSession DocumentSession { get; private set; }
    public CandidateService CandidateService { get; private set; }
    public ScriptService ScriptService { get; private set; }

    protected override void OnActionExecuting(ActionExecutingContext filterContext)
    {
        if (filterContext.IsChildAction)
        {
            return;
        }
        DocumentSession = Database.Instance.OpenSession();
        CandidateService = new CandidateService(DocumentSession);
        ScriptService = new ScriptService(DocumentSession);
        base.OnActionExecuting(filterContext);
    }
    
    protected override void OnActionExecuted(ActionExecutedContext filterContext)
    {
        if (filterContext.IsChildAction)
        {
            return;
        }
        if(DocumentSession != null)
        {
            if (filterContext.Exception == null)
            {
                DocumentSession.SaveChanges();
            }
            DocumentSession.Dispose();
        }
        base.OnActionExecuted(filterContext);
    }
}

This is all handled inside the base controller, and it is very similar to how I am doing that in my own apps.

However, ScriptService and CandidateService seems strange, let us explore them a bit.

public class ScriptService
{
    private readonly IDocumentSession documentSession;

    public ScriptService(IDocumentSession documentSession)
    {
        this.documentSession = documentSession;
    }

    public void AttachScript(string versionNumber, Stream fileContents)
    {
        var metadata = new RavenJObject();
        documentSession.Advanced.DatabaseCommands.PutAttachment(versionNumber, null, fileContents, metadata);
    }

    public Stream GetScript(string versionNumber)
    {
        var attachment = documentSession.Advanced.DatabaseCommands.GetAttachment(versionNumber);
        return attachment != null 
            ? attachment.Data() 
            : null;
    }
}

So this is using RavenDB attachment to store stuff, I am not quite sure what yet, so let us track it down.

This is being used like this:

[HttpGet]
public ActionResult GetScript(string versionNumber)
{
    var candidate = CandidateService.FindOneByVersionNumber(versionNumber);
    var attachment = ScriptService.GetScript(versionNumber);
    if (attachment != null)
    {
        var result = new FileStreamResult(attachment, "text/plain");
        var version = candidate.VersionNumber;
        var product = candidate.ProductName;
        result.FileDownloadName = string.Format("deploy-{0}-{1}.ps1", product, version);
        return result;
    }
    return new HttpNotFoundResult("Deployment script missing.");
}

So I am assuming that the scripts are deployment scripts for different versions, and that they get uploaded on every new release candidate.

But look at the CandidateService, it looks like a traditional service wrapping RavenDB, and I have spoken against it multiple times.

In particular, I dislike this bit of code:

public ReleaseCandidate FindOneByVersionNumber(string versionNumber)
{
    var result = documentSession.Query<ReleaseCandidate>()
        .Where(x => x.VersionNumber == versionNumber)
        .FirstOrDefault();
    if(result == null)
    {
        throw new ReleaseCandidateNotFoundException(versionNumber);
    }
    return result;
}

public void Store(ReleaseCandidate candidate)
{
    var existing = documentSession.Query<ReleaseCandidate>()
        .Where(x => x.VersionNumber == candidate.VersionNumber)
        .Any();
    if (existing)
    {
        throw new ReleaseCandidateAlreadyExistsException(candidate.VersionNumber);
    }
    documentSession.Store(candidate);
}

From looking at the code, it looks like the version number of the release candidate is the primary way to look it up. More than that, in the entire codebase, there is never a case where we load a document by id.

When I see a VersionNumber, I think about things like “1.0.812.0”, but I think that in this case the version number is likely to include the product name as well, “RavenDB-1.0.812.0”, otherwise you couldn’t have two products with the same version.

That said, the code above it wrong, because it doesn’t take into account RavenDB’s indexes BASE nature. Instead, the version number should actually be the ReleaseCandidate id. This way, because RavenDB’s document store is fully ACID, we don’t have to worry about index update times, and we can load things very efficiently.

Pretty much all of the rest of the code in the CandidateService is only used in a single location, and I don’t really see a value in it being there.

For example, let us look at this one:

[HttpPost]
public ActionResult MarkAsDeployed(string versionNumber, string environment, bool success)
{
    CandidateService.MarkAsDeployed(versionNumber, environment, success);
    return new EmptyResult();
}

As you can see, it is merely loading the appropriate release candidate, and calling the MarkAsDeployed method on it.

Instead of doing this needless, forwarding, and assuming that we have the VersionNumber as the id, I would write:

[HttpPost]
public ActionResult MarkAsDeployed(string versionNumber, string environment, bool success)
{
    var cadnidate = DocumentSession.Load<ReleaseCandidate>(versionNumber);
    if (cadnidate == null)
        throw new ReleaseCandidateNotFoundException(versionNumber);
    var env = DocumentSession.Load<DeploymentEnvironment>(environment);
    if (env == null)
        throw new InvalidOperationException(string.Format("Environment {0} not found", environment));

    cadnidate.MarkAsDeployed(success, env);
    return new EmptyResult();
}

Finally, a word about the error handling, this is handled via:

protected override void OnException(ExceptionContext filterContext)
{
    filterContext.Result = new ErrorResult(filterContext.Exception.Message);
    filterContext.ExceptionHandled = true;
}

public class ErrorResult : ActionResult
{
    private readonly string message;

    public ErrorResult(string message)
    {
        this.message = message;
    }

    public override void ExecuteResult(ControllerContext context)
    {
        context.HttpContext.Response.Write(message);
        context.HttpContext.Response.StatusCode = 500;
    }
}

The crazy part is that OnException is overridden only on some of the controllers, rather than in the base controller, and even worse. This sort of code leads to error details loss.

For example, let us say that I get a NullReferenceException. This code will dutifully tell me all about it, but will not tell me where it happened.

This sort of thing make debugging extremely hard.

Zen of python revisited with code samples

Roy Osherove — Wed, 16 May 2012 01:14:45 GMT

I really like reading this code. I will look deeper into it this week.

Many of the “zen” ideas here speak to me on the unit test level.