Peer Reviews – Why bother?

by Rasmus Kromann-Larsen January 03, 2012 23:58

Working with code is tricky business – the larger and more complex the code base, the more tricky. Ingraining micro-processes into your work day can help fix some of the issues, some of the broken windows that grow into almost any code base over time. Peer reviews is a great starting point.

There are many forms of peer review – but this post is mainly about informal check-in reviews. The process is simple: Any commit to the code repository must be signed off by another member of the team. Many argue that small commits are okay to go unchecked – but the size of a “small commit” grows in my experience. My counterargument is that a small commit will only take 30 seconds or less. Simply bring up the change set, go over the changes, discuss anything needed informally, then add “Review: <initials>” to the commit message and fire away.

Benefits?

One of the first things you’ll notice when you introduce peer reviews is catching common commit mistakes. These include small changes in files made while testing or debugging that were not meant to be committed, files that were not related to the current commit and, if your reviewer is alert, files missing from the commit.

Another small side effect is a subconscious increase in code quality. Knowing that someone else will review your code closely will increase the mental barrier to introducing hacks and other peculiarities that sometimes sneak into code.

While many developers focus more on writing self-documenting, readable code, getting another pair of eyes on the code is great for clarifying the intent of the code – uncovering small scale refactorings such as renames and extractions. The earlier you uncover and discuss minor design issues like these and further aligning team coding styles, the better shape the code base is likely to end up. Aligning coding styles across multiple teams is a hard task, any improvement is worth taking. Once in a while a peer review will uncover larger design issues and ultimately lead to discarding the code under review and going for a different solution. This is not always a pleasant experience, but it’s easier to kill your darlings when nudged in the right direction by a colleague.

In line with the last paragraph, reviews also often spur discussions about larger things like domain concepts and architecture – it just seems to come up more when looking at concrete issues in the code base. Likewise, the reviewer is investing some of this time in the code and putting his name on it, thus increasing shared code ownership of the code.

Lastly, just seeing how other developers work can give insight in other developers IDE and tool tricks. Being a keyboard-junkie myself, I often find myself exchanging IDE / productivity tips during reviews.

Conclusion

Information code reviews are, in my opinion, one of the cheaper ways to directly affect code quality – assuming it’s taken seriously of course. You might notice that many of these benefits are the same as with pair programming – and they are. Pair programming is usually harder to get started on and not suited for all assignments, although most teams ought to do way more pair programming than they are. Peer review is broadly applicable. Try it with your team for a week or a month – if I’m wrong and nothing improves, I’ll buy you a beer next time we meet :-)

kick it on DotNetKicks.com

Tags:

Check your backups – unexpected SQL Server VSS backup

by Rasmus Kromann-Larsen November 30, 2011 00:25

Your backup is only as good as your last restore. I recently changed my backup strategy on my SQL Server 2008 from doing a full nightly backup to doing incremental nightly backups and only a full backup each week. SQL Server incremental backups base themselves on the last full backup. This is nice when you go to restore them since you will only need the full backup + the incremental backup, not any intermediary backups.

However, a few days back I wanted to check some queries on a larger dataset and decided to check my backups at the same time. Fetched full + incremental backups from the server and started the local restore:

RESTORE DATABASE [testdb] 
FROM DISK = N'C:\temp\full.wbak' 
WITH FILE = 1, NORECOVERY, REPLACE

RESTORE DATABASE [testdb] 
FROM DISK = N'C:\temp\incremental.bak' 
WITH FILE = 1, RECOVERY

The first backup went through fine, but restoring the incremental backup resulted in the following error message:

This differential backup cannot be restored because the database has not been restored to the correct earlier state.

SQL Server refused to restore my incremental database – this is only supposed to happen if there has been another full backup in between. I double checked the backups I had fetched, checked that I had the set up the new backups correctly and that the old backup job was gone. Everything seemed fine.

I then explored the backup history a bit further with a query adjusted from the one found in this post:

SELECT TOP 10
s.database_name,
m.physical_device_name,
s.backup_start_date
FROM msdb.dbo.backupset s
INNER JOIN msdb.dbo.backupmediafamily m ON s.media_set_id = m.media_set_id
WHERE s.database_name = DB_NAME() -- Remove this line for all the database
ORDER BY backup_start_date DESC

The result showed that there had indeed been backups in between my nightly runs:

image

Further research revealed that backup devices with a GUID name are virtual backup devices and the times of backups matched the daily schedule of our bare metal system backup. Turns out that R1Soft's backup software integrates with SQL Server’s VSS writer service to perform backups when it finds databases on disk.

Disabling the VSS writer service returned the backups to a working state (VSS backup + my own incremental would also have worked). I did consider skipping my own nightly backups (since the VSS backup is super fast) and just using the R1Soft one, but decided against it for now – my own management is already set up and if I do need to restore, grabbing the backup from the external backup is much slower and more tedious than having it on disk already.

kick it on DotNetKicks.com

Tags:

NHibernate Flushing and You

by Rasmus Kromann-Larsen June 15, 2011 01:47

Working with NHibernate, you will eventually have to know something about flushing. Flushing is the process of persisting the current session changes to the database. In this post, I will explain how flushing works in NHibernate, which options you have and what the benefits and disadvantages are.

As you work with the NHibernate session, loading existing entities and attaching new entities, NHibernate will keep track of the objects that are associated with current session. When a flush is triggered, NHibernate will perform dirty checking; inspect the list of attached entities to determine what needs to be saved and which SQL statements are required to persist the changes.

NHibernate offers several different flush modes that determine when a flush is triggered. The flush mode can be set using a property on the session (usually when opening the session).

Out of the box NHibernate defaults to FlushMode.Auto which is a flush mode that offers a minimum of surprises while providing decent performance. Auto will flush changes to the database when a manual flush is performed (using ISession.Flush()), when a transaction is committed and when NHibernate deems that an auto flush is necessary to serve up-to-date results in response to queries. While the auto flush is convenient, it does cause a few disadvantages as well. To determine whether an auto flush is required before executing a query NHibernate has to inspect the entities attached to the session. This is clearly a performance overhead and unfortunately as application complexity (and thus likely session length, number of queries and number of attached entities) increases, the cost will be in the ballpark of O(q*e) – quadratic growth based on number of queries and entities. Furthermore auto flushes are not always easy to predict, especially in complex systems – this can lead to unexpected exceptions if using things like NHibernates merge and replicate features (a blog post all by itself).

A better solution for bigger applications is FlushMode.Commit, this flush mode will flush on manual flushes and when transactions are committed. Avoiding auto flushes provides quite a few performance opportunities, it will potentially require fewer SQL statements (multiple changes to the same data), it will cause fewer round trips to the database and thus enable better batching. What you need to understand before changing your flush mode to FlushMode.Commit is that your queries may return stale results until you commit transactions. However, this is generally what people expect when working with transactions, so it is usually not a problem. In some cases, you might have to perform a manual flush, but it makes sense to reduce the number of these (since they defeat the benefit of the flush mode).

NHibernate offers two more (usable) flush modes. FlushMode.Always will trigger a flush before every query and is thus generally not useful except for maybe some special edge cases. FlushMode.Never will cause the session to only flush when manually flushing – this can be useful to create a read-only session (better performance and more assurance that no flushes are performed). For read-only / bulk needs, it’s also practical to look into IStatelessSession (low memory / performant for bulk operations) and ReadOnly on queries and criterias introduced in NHibernate 3.1.

kick it on DotNetKicks.com

Tags:

Countdown timer

by Rasmus Kromann-Larsen May 26, 2011 22:38

I demoed a small app today at the Demo Dag session at Community Day.

The app was developed at an ANUG Code Dojo - and the purpose is simply to create timers that are a few pixels high either at the bottom or top of your screen - to be used for running Pomodoros or other timing needs - like an informal timer for a presentation.

I got a few requests for the app, so I've uploaded the source to Bitbucket here (there's also a v. 0.1 zip file with an executable - if you don't want to build from source). 

Bear in mind that this app was hacked together in a few hours (with the purpose of learning WPF actually - we got sidetracked) - so don't expect quality code or an excellent polished app. It has quirks - you have been warned :-)

image

Enjoy.

kick it on DotNetKicks.com

Tags:

Slides from ANUG VS Launch Event

by Rasmus Kromann-Larsen May 24, 2010 12:53

I spoke last week about ReSharper 5 at ANUG’s Visual Studio 2010 launch event.

Here are the slides from my presentation. The slides are in danish and probably won’t make too much sense as most of my presentation was done demoing stuff – but they should give the gist of it.

Slides

If you have any questions on my presentation, feel free to shoot me a mail here on the blog :-)

kick it on DotNetKicks.com

Tags:

Black / Blue Visual Studio 2010 + ReSharper 5 Theme

by Rasmus Kromann-Larsen April 20, 2010 23:15

I have been using black background in Visual Studio for as long I can remember. I started out using Rob Conery’s black / orange TextMate theme, but last year I created my own black theme with a blueish style. Today at the ANUG code dojo we tweaked it to actually look alright with the changes in Visual Studio and especially ReSharper.

You can download the theme here if you want a nice black theme.

It looks like this:

theme

Selection:

image

Subtle highlights of active identifier:

image

Enjoy :-)

kick it on DotNetKicks.com

Tags:

Slides from Miracle Open World

by Rasmus Kromann-Larsen April 20, 2010 22:52

Last week I gave two talks at MOW2010. Was an awesome conference and the 80% talks + 80% networking concept really held true. Hope to be going again next year – as speaker or otherwise. Here is the slides from my two talks.

Increasing productivity with ReSharper


This talk is about optimizing the mechanical part of your work. See how a keyboard-centric focus can speed
up your work and how to navigate codebases easily independent of size. Visual Studio 2010 has introduced more
advanced keyboard features, but ReSharper is still king, so it will be the main focus of the talk. While this
session will contain a lot of fast-paced flashy keyboard shortcuts, it will also contain basic techniques and
advice for you to get started with your own keyboard.

Slides

Practical ASP.NET MVC 2


ASP.NET MVC is the new kid on the Microsoft block. This talk will give you a short introduction to the framework
and the new features in ASP.NET MVC 2. After the introduction, we will dig into some practical experiences and
common situations of actually implementing a system using ASP.NET MVC. Detours will include other alternative
open-source web frameworks and maybe even some JavaScript.

Slides

kick it on DotNetKicks.com

Tags:

SOLID Presentation Slides

by Rasmus Kromann-Larsen October 27, 2009 21:20

A few weeks back I gave a talk in Odense .NET User Group (ONUG)  on “Practical SOLID in C#” about object-orientation and the SOLID principles.

Here is the slide deck for the presentation.

Download

kick it on DotNetKicks.com

Tags:

Why AAA style mocking is better than Record / Playback

by Rasmus Kromann-Larsen May 24, 2009 21:06

If you've followed me on twitter for more than a couple of days, you will most probably have heard my grumbling each time I run into issues using record / playback mocking - so I thought I'd write a short post on my experiences with both and why I think that I keep bumping into issues with record / playback.

Phases of a test

If you take a look at the normal flow of a test method without mocking, it will usually perform some kind of setup, then perform some action that invokes the code under test - and then at the end, make some assertions about the state of some component that you want verified. While some tests continue after this point, this is where you should stop if you're following the "one assert per test" rule - this is sort of the single responsibility principle for tests. This flow is also the inspiration for the AAA name - first you Arrange your test setup, then you Act upon the class under test - and then you Assert something about the state. Here's a simple example without mocking:

[Test]
public void CanRemoveCategories()
{
    // Arrange
    var collection = new CategoryCollection("test");

    // Act
    collection.RemoveCategory("test");

    // Assert
    Assert.That(collection.Count, Is.EqualTo(0));
}

This test is chronologically sound, it makes sense and it is easy to read - but then again, this is a state-based test, I said I was going to talk about behavioral tests - mock tests.

Phase confusion

Many people find that mocking is rather difficult to understand and that it often very hard to read and understand. Since our tests act as an API-description for our code - and since we want to be able to figure out how to fix our failed tests - readability is important. Now, let's look at one of my tests from a Record / Playback point of view. I'm using Rhino Mocks as my mocking framework in this test:

[Test]
public void ShouldIgnoreRespondentsThatDoesNotExistRecordPlayback()
{
    var guid = Guid.NewGuid();
    IEventRaiser executeRaiser;

    using(_mocks.Record())
    {
        Expect.Call(_view.Respondents).Return(new[] {guid.ToString()});
        Expect.Call(_repository.GetById(guid)).Return(null);

        _view.ExecuteOperation += null;
        executeRaiser = LastCall.IgnoreArguments()
            .Repeat.Any()
            .GetEventRaiser();

        Expect.Call(_view.OperationErrors = null)
            .IgnoreArguments()
            .Constraints(List.IsIn("Non-existant respondent: " + guid));
    }

    using(_mocks.Playback())
    {
        new BulkRespondentPresenter(_view, _repository);
        executeRaiser.Raise(null, EventArgs.Empty);
    }
}

Now, at a glance, can you tell me what this test is really doing? There's something with a view and a repository and we can probably deduce quite a bit from the test name. But it's rather hard to separate the different phases I talked about before Arrange, Act and Assert. Below, I've tried to annotate the test with the phases:

[Test]
 public void ShouldIgnoreRespondentsThatDoesNotExistRecordPlayback()
 {
     // Arrange
     var guid = Guid.NewGuid();
     // Part of Act
     IEventRaiser executeRaiser;

     using(_mocks.Record())
     {
         // Arrange (or Assert?)
         Expect.Call(_view.Respondents).Return(new[] {guid.ToString()});
         Expect.Call(_repository.GetById(guid)).Return(null);

         // Part of Act
         _view.ExecuteOperation += null;
         executeRaiser = LastCall.IgnoreArguments()
             .Repeat.Any()
             .GetEventRaiser();

         // Assert
         Expect.Call(_view.OperationErrors = null)
             .IgnoreArguments()
             .Constraints(List.IsIn("Non-existant respondent: " + guid));
     }

     using(_mocks.Playback())
     {
         // Arrange
         new BulkRespondentPresenter(_view, _repository);
         // Act
         executeRaiser.Raise(null, EventArgs.Empty);
     }
 }

No wonder it's hard to read and understand. The phases are mixed all over - and the Asserts are in the middle of the test - this is nothing like the natural flow of the previous test without mocking. I usually like to have the phases separated in my test with comments as well, but it's just not possible in this test. I wrote this test up rather quickly, so there might be a better way of doing it that I am missing - if there is, please yell at me :-)

Sorting out the confusion

AAA mocking, as the name suggests, is all about clearing out the confusion in that last test - it's about maintaining the original test flow. It just so happens also to have some other benefits, that I will get into later in the post. I've written the same test as above in an AAA style, this time with Moq, since I'm trying it out at the moment, but Rhino Mocks has similar syntax. Moq is pretty heavy on lambda expressions, but even if you haven't worked with those yet, I'm sure you will grasp the idea. If you want a general introduction to mocking with Moq, Justin Etheredge has a small series about it.

[Test]
public void ShouldIgnoreRespondentsThatDoesNotExist()
{
    // Arrange
    var guid = Guid.NewGuid();
    _viewMock.Setup(x => x.Respondents).Returns(new[] { guid.ToString() });
    _repositoryMock.Setup(x => x.GetById(guid)).Returns(() => null);

    // Act
    _viewMock.Raise(x => x.ExecuteOperation += null, EventArgs.Empty);

    // Assert
    _viewMock.VerifySet(x => x.OperationErrors =
        It.Is<IList<string>>(l => l.Contains("Non-existant respondent: "+guid)));
}

Those comments are actually in my original test as well - and in my test live template I generate all my tests with. If you compare this test to the one above, you will see that it has more or less the same components, but this time, they're arranged in a way that makes sense for the next reader of the test. The fact that the test is shorter is also slightly unfair, since my first test used an event raiser, which involves "many" lines of code. Also the separation of the phases allowed me to move the actual construction of the presenter our of the actual test and into shared setup code.

So what techniques did AAA mocking introduce to help alleviate the pains? First of all, the mocks no longer has states - that's what record and playback really refer to: A mock in record state will record calls made on it and then expect them to be called again during the playback state. Furthermore, it cleanly separates mock setup from mock expectations.

What is gained?

So what did we gain with AAA style mocking over the traditional record / playback style?

  • The main selling point for me is readability and test simplicity - it is much easier for me to explain mocking to someone else with AAA.
  • If you have done any fairly advanced record / playback mocking, you will find that when the mocks have states, it will often result in subtle test failures.
  • Clean separation of test phases.
  • Greatly improved ability to move shared code out of tests. Since you have the first part of your test handling setup, extending this part to start before the actual test (in a setup method) is no problem. With record / playback mocking, you will often run into state failures if you attempt it.

kick it on DotNetKicks.com

Tags:

Development | Testing

Be Mindful Of Your Dependencies

by Rasmus Kromann-Larsen May 11, 2009 20:35

Dependencies are everywhere in your code, you cannot avoid them, without dependencies, your software would make no sense, it is the interaction and collaboration between the software entities that create the program. Now this statement doesn't mean that we should not worry about our dependencies - we just can't remove them completely. But as with almost any other aspect of good software development, we should be mindful of our dependencies.

Coupling

When a software entity depends on another, they are said to be coupled. Coupling ranges in degrees from totally decoupled over loosely coupled to tightly coupled. The tighter two software entities are coupled, the bigger risk that when one changes, the other will be forced to change as well. Why is this important? Because coupling is transitive (if A depends on B and B depends on C, A indirectly depends on C) and we would like to be able to change our program in the future. If you have a program where all components depend on each other and the coupling is high, a small change will often ripple through the system - either a) forcing you to change a bigger part of the system (potentially creating more ripples) or b) breaking the system in unexpected places. If not treated properly, these ripples can cause developers to loose confidence in their changes (what will I break this time?) and slow development down - maybe even to a halt.

Cohesion

Cohesion describes how related the responsibilities within a component are, how focused it is. This relates to the functionality within the component - a static utility class will often have rather low cohesion. But it also relates to the level of abstraction used within the component - again, a class that does both very high level operations and very low level operations will often have very low cohesion. Code with low cohesion will often be harder to understand, since it doing many different things. It will often be easier to reuse a component with high cohesion, since it will be more focused on doing a single task.

Managing Dependencies

So how do we manage dependencies? As mentioned, we can't get rid of them - but we can choose to decouple our software components from each other - and we can work to increase the cohesion of our components. Looking into the Gang of Four book, some basic advice is available:

  • Program to an interface, not an implementation.
  • Favor composition over inheritance.

Interfaces

Decoupling often involves programming to an interface instead of an implementation. Interfaces can help us break our dependency chains - looking at the example from before: If A depends on B and B depends on C - we can break the dependency chain by making an interface IB of B. Now A depends on IB instead. Interfaces, when used properly, can be seen as concepts and are thus more "stable" than an actual implementation. It is a contract that describes a set of properties for some object to fulfill. If you were to build a tall tower, would you prefer to have the core of the tower made of stable building blocks or instable ones?

Having an interface also allows us to tweak other things, such as the granularity of access - the size of the surface on which A is dependant. If we reduce the size of the interface and thus the dependency, A's usage of B will be easier to control - and new concrete classes based on IB will probably be easier to implement and have higher cohesion too.

Composition

Another factor that can help us reduce coupling is to favor composition over inheritance. I like the analogy from the Gang of Four book where they talk about inheritance as white-box reuse, while composition is black-box reuse. What this means is that a sub-class will often be tightly coupled to the internals of the parent. In languages like C# and Java, single inheritance limits your options even further, once you start inheriting - and languages with multiple inheritance often suffer from other problems.

Composition on the other hand, is all about creating small units of focused behavior and then "weaving" this behavior into the desired functionality. With composition, different components are free to vary independent of each other - whereas this might cause combinatorial explosions in inheritance trees.

This is not to say that inheritance is not useful, but it is often overused - composition is often a little harder to grasp at first (I know it took me a while), but it is a really powerful technique.

Conclusion

In this post I have discussed some of the basics about managing dependencies. It is on a reasonably low level, but will be the basis for some of my next posts on design principles and design patterns - since many of them build upon exactly this.

kick it on DotNetKicks.com

Tags:

Design

Powered by BlogEngine.NET 1.6.1.0
Theme by Mads Kristensen | Modified by Mooglegiant | Adjusted by Rasmus Kromann-Larsen

About Me

I am a danish .NET developer blogging about the technical side of my life, mostly .NET stuff, but also fundamental topics like design patterns, principles and productivity boosters.

In addition, I am a core group member of the two largest .NET user groups in Denmark: CNUG and ANUG.